https://pantsbuild.org/ logo
Join Slack
Powered by
Hey guys! We recently tried upgrading from `1.13....
# general
c
Hey guys! We recently tried upgrading from 
1.13.0
to 
1.14.0
and one of our supported plugins (forked from 
fsqio
) failed with 
<class 'fsqio.pants.spindle.tasks.build_spindle.BuildSpindle_build_spindle'> must set an options_scope class-level property
More details Bug happens between transition from 
1.14.0dev1
to 
1.14.0dev2
Seems like we’re missing setting related to 
optionable.py
(https://github.com/pantsbuild/pants/blob/d9eb4403d551df3c3fe107cbdc272f169ec7599e/src/python/pants/option/optionable.py#L140):
Copy code
cls = type(self)
if not isinstance(cls.options_scope, str):
  raise NotImplementedError('{} must set an options_scope class-level property.'.format(cls))
- adding 
options_scope
val to class didn’t help - adding custom 
init
to class that prints related to the issue lines and calls super outputs this:
Copy code
type(self): <class 'fsqio.pants.spindle.tasks.build_spindle.BuildSpindle_build_spindle'>
type(self).options_scope: build-spindle
isinstance(type(self).options_scope, str): True
last parts of the stack trace
Copy code
File "/Users/michael/.cache/pants/setup/bootstrap-Darwin-x86_64/1.14.0/lib/python2.7/site-packages/pants/engine/round_engine.py", line 42, in attempt
    task = task_type(self._context, task_workdir)
  File "/Users/michael/sigma_project/sigma-monorepo/src/python/fsqio/pants/spindle/tasks/build_spindle.py", line 58, in __init__
    super(BuildSpindle, self).__init__(*args, **kwargs)
  File "/Users/michael/.cache/pants/setup/bootstrap-Darwin-x86_64/1.14.0/lib/python2.7/site-packages/pants/backend/jvm/tasks/nailgun_task.py", line 71, in __init__
    super(NailgunTaskBase, self).__init__(*args, **kwargs)
  File "/Users/michael/.cache/pants/setup/bootstrap-Darwin-x86_64/1.14.0/lib/python2.7/site-packages/pants/task/task.py", line 690, in __init__
    super(Task, self).__init__(context, workdir)
  File "/Users/michael/.cache/pants/setup/bootstrap-Darwin-x86_64/1.14.0/lib/python2.7/site-packages/pants/task/task.py", line 173, in __init__
    super(TaskBase, self).__init__()
  File "/Users/michael/.cache/pants/setup/bootstrap-Darwin-x86_64/1.14.0/lib/python2.7/site-packages/pants/option/optionable.py", line 151, in __init__
    raise NotImplementedError('{} must set an options_scope class-level property.'.format(cls))

Exception message: <class 'fsqio.pants.spindle.tasks.build_spindle.BuildSpindle_build_spindle'> must set an options_scope class-level property.
^ without custom init 
super(BuildSpindle, self).__init__(*args, **kwargs)
is just skipped from the output but overall trace stays the same As codebase is forked and works fine for out purposes we usually try to avoid changes unless it breaks so it might be that underlying issue was there before and just was brought to surface with recent update, still if there any ideas what might affect it’d be great to know, version of the referred file that we forked from (and haven’t changed much since except for small tweaks): https://github.com/foursquare/fsqio/blob/master/src/python/fsqio/pants/spindle/tasks/build_spindle.py 🙏🏼 for your help and guidance
h
Hm, what happens when you add something like https://github.com/pantsbuild/pants/blob/master/src/python/pants/backend/python/subsystems/pytest.py#L11? That’s how we do it everywhere in the Pants repo so I’m surprised/concerned it didn’t work here
c
same error, in message above I print output that class has
Copy code
type(self): <class 'fsqio.pants.spindle.tasks.build_spindle.BuildSpindle_build_spindle'>
type(self).options_scope: build-spindle
isinstance(type(self).options_scope, str): True
so 
options_scope
is not only defined but proved to be set there
@hundreds-father-404 does it make sense?
h
Yes - trying to figure out what’s going on
c
🙏🏼
h
To help debug, what happens if you add 
options_scope
to 
SpindleTask
? Also, what does your custom 
__init__
look like for 
BuildSpindle
?
c
Copy code
def __init__(self, *args, **kwargs):
    print("type(self): {}".format(type(self)))
    print("type(self).options_scope: {}".format(type(self).options_scope))
    print("isinstance(type(self).options_scope, str): {}".format(isinstance(type(self).options_scope, str)))
    super(BuildSpindle, self).__init__(*args, **kwargs)
yeah I already added 
options_scope
there as well JIC
but it doesn’t use one from 
SpindleTask
as it’s getting properly overriden by 
BuildSpindle
h
Oof I think I maybe figured it out… do you have the 
future
library installed? When running with Python 2, that library override’s the builtin 
str
to be the same as Python 3's unicode str. I’ll run a quick test to see if this indeed is the issue
c
yeah, i do import 
unicode_literals
h
In your custom init, please add this as a test:
Copy code
isinstance(b"test", str)
isinstance(u"test", str)
c
Copy code
True
False
h
Okay that confirms my suspicion. When in your custom init, you’re asserting 
isinstance(options_scope, str)
that means you’re asserting it’s really bytes. With our use of the future library, our check is asserting it’s really unicode. It’s confusing because Pants / the future library overwrites what the builtin 
str
means. So, this means the property is being stored as bytes, which makes sense because this is how Python 2 stores properties. It’s an issue with our code that I will hotfix before we release 1.15.0. I’m trying to think of a workaround for you in the meantime..
c
should I encode 
options_scope
in some manner maybe?
h
That’s what I’m trying to figure out. So what’s going on is that Python stores properties as a dictionary between the property name to its value. The value of 
options_scope
is being stored as unicode like it should, but the key in the dictionary, i.e. 
options_scope
, is being stored as bytes because Python 2 works that way. We need some way to get Python to store the key as unicode.. trying to think if there’s a way to monkey patch that
I’ll keep thinking about this tonight but at the moment don’t think this is possible 😕 working on a fix as we speak for 1.15.0, which should be released in the next week or two I’m sorry about this mistake!
c
no worries, ty for the update and quick response! just to be clear, can I somehow port pants future library to our code base?
not quite sure how big of the module is it and if it’ll work
Pants 1.14.0 is still Python2, right?
h
You can install 
future==0.17.1
, but I’m not sure it will fix things. I’m still trying to figure out how this change didn’t break Pants code - we run our integration tests with Python 2 every night, so those should have failed 🤔 -- Yes. And Pants 1.15.0 will be Python 2.7 or 3.6, Pants 1.16.0 will be 2.7, 3.6, or 3.7, and 1.17.0 will be 3.6 or 3.7.
👍🏼 1
c
with future update and 
from builtins import str, super
I got to
Copy code
False
True
and init now outputs: 
isinstance(type(self).options_scope, str): False
so yeah, progress but not very helpful I guess
h
Cool, that’s good. Think it will continue to fail unless there’s a way to override how Python stores the property name, unfortunately.
c
makes sense, I’ll wait for hot-fix and 1.15.0 then ty so much!
h
(Btw, despite this bad first introduction to the library, the future library is awesome. It allows you to write Python 3 code that also works with Python 2. Better version of Six. If you’re migrating any of your code to Python 3, I highly recommend using future)
💯 1
https://github.com/pantsbuild/pants/pull/7467 Thank you for discovering this and the very descriptive message! You made this much easier to debug, and I appreciate that. I’ll see if we can release 1.14.1 in addition to including this in 1.15.0 (which should come out this week or next)
👖 1
🦜 1
Wait actually @curved-vase-73456, can you please post a pastebin or gist of what your class definition looks like? Along with 
print(type(self).options_scope)
c
sure, give me 5 mins
h
Rather than 
print(type(self).options_scope)
, try 
repr(type(self).options_scope)
. 
print()
strips the 
u''
prefix And then change to 
options_scope = u"build-spindle"
as a sanity check
This is what should be going on.
Copy code
pex --python=python2.7 future==0.17.1
Python 2.7.15 (default, Jan 14 2019, 13:17:44)
[GCC 4.2.1 Compatible Apple LLVM 10.0.0 (clang-1000.10.44.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from builtins import str
>>> class Test(object):
...   options_scope = u"test"
...
>>> isinstance(Test.options_scope, str)
True
>>> isinstance(type(Test()).options_scope, str)
True
c
hm, still have 
False
there, even though 
u'type(self).options_scope: build-spindle'
h
Don’t include the prefix 
type(self).options_scope:
in the 
repr()
call, because the call to 
.format()
will remove the 
u''
prefix
c
just gives then 
'build-spindle'
h
Which confirms for some reason the value associated with 
options_scope
is being stored as bytes. Weird Realized thanks to John’s comment on that PR that what I was saying earlier about the property name is irrelevant. The issue is the property’s value, not the name of the property
One other test, in the class definition use: 
options_scope = b"test".decode('utf-8')
c
yeah, same
oh, interesting it seems I cannot override 
options_scope
, it’s 
build-spindle
no matter what I do
😶 1
so 
options_scope
value comes somehow from task name in 
register.py
task(name='build-spindle', action=BuildSpindle).install()
so adding 
u
fixed it, i.e changing to 
task(name=u'build-spindle', action=BuildSpindle).install()
¯\_(ツ)_/¯
h
Does the file not have unicode_literals in it? Glad we got this figured out!!
c
yeah 
register.py
didn’t 😞 wrong imports omg, ty for going though this with me
❤️ 1
3 Views
Open in Slack
PreviousNext
Powered by