Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix click regression #526

Merged
merged 1 commit into from
May 13, 2021
Merged

Fix click regression #526

merged 1 commit into from
May 13, 2021

Conversation

oavdeev
Copy link
Collaborator

@oavdeev oavdeev commented May 13, 2021

The problem

Click 8.0 has been released yesterday, and upgrading it seems to break IncludeFile and JSON parameter parsing.

For IncludeFile the error looks like this:

Traceback (most recent call last):
  File "/Users/oleg/src/metaflow/metaflow/cli.py", line 987, in main
    start(auto_envvar_prefix='METAFLOW', obj=state)
  File "/Users/oleg/mfci/lib/python3.7/site-packages/click/core.py", line 1134, in __call__
    return self.main(args, kwargs)
  File "/Users/oleg/mfci/lib/python3.7/site-packages/click/core.py", line 1059, in main
    rv = self.invoke(ctx)
  File "/Users/oleg/mfci/lib/python3.7/site-packages/click/core.py", line 1663, in invoke
    sub_ctx = cmd.make_context(cmd_name, args, parent=ctx)
  File "/Users/oleg/mfci/lib/python3.7/site-packages/click/core.py", line 927, in make_context
    self.parse_args(ctx, args)
  File "/Users/oleg/mfci/lib/python3.7/site-packages/click/core.py", line 1376, in parse_args
    value, args = param.handle_parse_result(ctx, opts, args)
  File "/Users/oleg/mfci/lib/python3.7/site-packages/click/core.py", line 2352, in handle_parse_result
    value = self.process_value(ctx, value)
  File "/Users/oleg/mfci/lib/python3.7/site-packages/click/core.py", line 2308, in process_value
    value = self.type_cast_value(ctx, value)
  File "/Users/oleg/mfci/lib/python3.7/site-packages/click/core.py", line 2295, in type_cast_value
    return convert(value)
  File "/Users/oleg/mfci/lib/python3.7/site-packages/click/types.py", line 75, in __call__
    return self.convert(value, param, ctx)
  File "/Users/oleg/src/metaflow/metaflow/includefile.py", line 230, in convert
    value = os.path.expanduser(value)
  File "/usr/local/Cellar/[email protected]/3.7.10_3/Frameworks/Python.framework/Versions/3.7/lib/python3.7/posixpath.py", line 235, in expanduser
    path = os.fspath(path)
TypeError: expected str, bytes or os.PathLike object, not function

Solution

It looks like the reason is that convert() method on custom click ParamTypes is supposed to be idempotent.

This must accept string values from the command line, as well as values that are already the correct type.

Metaflow's implementation didn't do that. This requirement was mentioned in click documentation for earlier versions as well, but it looks like it didn't matter in practice until 8.0.

@savingoyal savingoyal merged commit f7ae407 into Netflix:master May 13, 2021
@oavdeev oavdeev deleted the fix-click branch May 13, 2021 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants