TypeError: While assigning key 'base_name': Expected u'bot_update - 58.0.3029.33' to be of type <type 'str'>

[Paweł Hajdan, Jr.]

I've been trying to use "suffix" parameter of bot_update.ensure_checkout for publish_tarball - see https://chromium-review.googlesource.com/c/469727/ .

I'm getting a failure: https://luci-milo.appspot.com/buildbot/chromium.infra.cron/publish_tarball/6720 : https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.infra.cron%2Fpublish_tarball%2F6720%2F%2B%2Frecipes%2Fsteps%2FUncaught_Exception%2F0%2Flogs%2Fexception%2F0 :

Traceback (most recent call last): File "/b/rr/tmpfaeHZS/rw/recipes/.recipe_deps/recipe_engine/recipe_engine/run.py", line 316, in _new_run recipe_result = recipe_script.run(api, properties) File "/b/rr/tmpfaeHZS/rw/recipes/.recipe_deps/recipe_engine/recipe_engine/loader.py", line 81, in run self.run_steps, properties, self.PROPERTIES, api=api) File "/b/rr/tmpfaeHZS/rw/recipes/.recipe_deps/recipe_engine/recipe_engine/loader.py", line 580, in invoke_with_properties **additional_args) File "/b/rr/tmpfaeHZS/rw/recipes/.recipe_deps/recipe_engine/recipe_engine/loader.py", line 541, in _invoke_with_properties return callable_obj(*props, **additional_args) File "/b/rr/tmpfaeHZS/rw/recipes/recipes/publish_tarball.py", line 188, in RunSteps api.bot_update.ensure_checkout(with_branch_heads=True, suffix=version) File "/b/rr/tmpfaeHZS/rw/recipes/.recipe_deps/recipe_engine/recipe_engine/recipe_api.py", line 578, in _inner return func(*a, **kw) File "/b/rr/tmpfaeHZS/rw/recipes/.recipe_deps/depot_tools/recipes/recipe_modules/bot_update/api.py", line 264, in ensure_checkout ok_ret=(0, 87, 88), **kwargs) File "/b/rr/tmpfaeHZS/rw/recipes/.recipe_deps/recipe_engine/recipe_engine/recipe_api.py", line 578, in _inner return func(*a, **kw) File "/b/rr/tmpfaeHZS/rw/recipes/.recipe_deps/depot_tools/recipes/recipe_modules/bot_update/api.py", line 46, in __call__ return self.m.python(name, bot_update_path, cmd, **kwargs) File "/b/rr/tmpfaeHZS/rw/recipes/.recipe_deps/recipe_engine/recipe_engine/recipe_api.py", line 578, in _inner return func(*a, **kw) File "/b/rr/tmpfaeHZS/rw/recipes/.recipe_deps/recipe_engine/recipe_modules/python/api.py", line 25, in __call__ return self.m.step(name, cmd + list(args or []), **kwargs) File "/b/rr/tmpfaeHZS/rw/recipes/.recipe_deps/recipe_engine/recipe_modules/step/api.py", line 228, in __call__ schema.set_val(kwargs) File "/b/rr/tmpfaeHZS/rw/recipes/.recipe_deps/recipe_engine/recipe_engine/config.py", line 430, in set_val raise type(e)('While assigning key %r: %s' % (name, e)) TypeError: While assigning key 'base_name': Expected u'bot_update - 58.0.3029.33' to be of type <type 'str'>

1. The immediate issue seems to be unicode vs str. What are your recommendations how to solve that - specifically, what's the best place to modify? I see at least the following alternatives:

1a. Allow unicode for base_name. Looks like there might be buildbot compatibility issues though (recipe_modules/step/config.py):

    # For compatibility with buildbot, the step name must be ascii, which is why
    # this is a 'str' and not a 'basestring'.
    name = Single(str),
    base_name = Single(str, required=False),

1b. Convert to str in bot_update.

1c. Convert to str in publish_tarball.

2. Why wasn't this detected at recipe simulation stage?

Paweł

[d...@google.com]

> 1. The immediate issue seems to be unicode vs str. What are your recommendations how to solve that - specifically, what's the best place to modify? I see at least the following alternatives:
>
> 1a. Allow unicode for base_name. Looks like there might be buildbot compatibility issues though (recipe_modules/step/config.py):
> # For compatibility with buildbot, the step name must be ascii, which is why
> # this is a 'str' and not a 'basestring'.
> name = Single(str),
> base_name = Single(str, required=False),

Yeah, BuildBot probably wouldn't be too happy with this. I've added unicode support in some places to BuildBot as an emergency fix, but it's always a bolt-on hack, and constraining step name seems reasonable.

>
> 1b. Convert to str in bot_update.
>
> 1c. Convert to str in publish_tarball.

Probably a good idea?

> 2. Why wasn't this detected at recipe simulation stage?

Venturing a guess: the circumstances under which the unicode string was generated aren't part of any "GenTests" simulation?

[Robert Iannucci]

Yeah, that's about right.

This is caused by the engine loading properties from disk using python's native 'json.load' method, which really likes the unicode string type. Over the years folks have accumulated workarounds (something like make_it_work(str(unicode(val.encode('utf-8', 'replace'))).voodoo()) ) in various recipes and stuff to work around this. It doesn't show up in simulation tests when the user uses `api.properties(val='foo')` in the test; val is now the `str` type. This has also lead to Configs needing to use basestring or (str, unicode), or other hacks like that. The philosophy in python2 should always be: convert unicode objects to utf-8 encoded string (by doing unicode.encode('utf-8', 'replace')), and assume that str means 'a series of utf-8 encoded bytes'. The reason for this is that python2's internal and external support for the unicode type varies from 'basic' to 'awful'. In fact the way that python2 handles unicode is one of the major reasons that python3 isn't backwards compatible with python2[1]. The rationale snippet here is: "To be prepared in Python 2.x, start using unicode for all unencoded text, and str for binary or encoded data only". We know with 100% certainty that JSON is utf8 (it's part of the JSON spec), which is why it's safe to encode the 'unicode' objects that we get back from json.loads to utf8.

Pawel and I just chatted about this and he's going to look into fixing the way recipe engine loads properties from disk (to apply the same unicode.encode('utf-8', 'replace') logic that the api.json.load function uses), and to change the recipe engine's default encoding to utf-8[2]. Once this is done, '💩'.encode('utf-8') == u'💩', which is obviously the Most Important Thing ;).  I was meaning to do this a week or so ago and I lost track of it[3] :/.

Mostly kicking myself for not doing this from day 1. Live and learn, I guess.

[1]: https://docs.python.org/3.0/whatsnew/3.0.html#text-vs-data-instead-of-unicode-vs-8-bit

[2]: Which requires a little bit of a hack (but it's for a good cause):

import sys
reload(sys)
sys.setdefaultencoding('UTF8')

[3]: Actually I got stuck trying to convince json.loads and the generated python protobuf code to return encoded str's for strings, but they both have the same unicode disease: not only is there no supported way to override them, there's not even a kludgey way to do it when they're using the C runtimes (the python json.load implementation actually DOES have an undocumented way to override it by providing a different API than it's supposedly-identical C implementation... *sigh*). I was trying to take this approach because those are the only two sources of unicode objects in the entire recipes ecosystem... I was hoping to fix them at the source, but there's only awful ways to do it for json.load, and essentially no real way to do it for generated protobuf code.

--
You received this message because you are subscribed to the Google Groups "infra-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+...@chromium.org.
To post to this group, send email to infr...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-dev/135f95a3-6f66-43c8-9ad6-493ce46add93%40chromium.org.

[Paweł Hajdan, Jr.]

Thanks!

I landed https://chromium-review.googlesource.com/c/470226 as immediate publish_tarball workaround, and uploaded https://codereview.chromium.org/2798393002/ for the recipe engine (which I plan to land tomorrow so I can monitor).

Paweł

To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+unsubscribe@chromium.org.

To post to this group, send email to infr...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-dev/135f95a3-6f66-43c8-9ad6-493ce46add93%40chromium.org.

--

You received this message because you are subscribed to the Google Groups "infra-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+unsubscribe@chromium.org.

To post to this group, send email to infr...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-dev/CA%2Bq_oBc0Z9XbH6YDDmaA5a_hBzb3u7fVuMENCqmqh5vP2zP98g%40mail.gmail.com.

[Robert Iannucci]

👍👍👍👍

To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+...@chromium.org.

To post to this group, send email to infr...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/infra-dev/135f95a3-6f66-43c8-9ad6-493ce46add93%40chromium.org.

--
You received this message because you are subscribed to the Google Groups "infra-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to infra-dev+...@chromium.org.

To post to this group, send email to infr...@chromium.org.