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

Don't raise error if metaflow_extension's Distribution#files is None. #2091

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

trhodeos
Copy link
Contributor

@trhodeos trhodeos commented Oct 9, 2024

With the metaflow-card-html package, we're seeing the following error:

.../metaflow/extension_support/__init__.py", line 414, in _get_extension_packages
    for f in dist.files:
TypeError: 'NoneType' object is not iterable

Turns out Distribution.files can return None. metaflow-card-html doesn't include the appropriate wheel metadata files, and, when included, hits this error.

@trhodeos trhodeos changed the title Don't raise error if metaflow_extension plugin with None files is encountered. Don't raise error if metaflow_extension's Distribution#files is None. Oct 9, 2024
savingoyal
savingoyal previously approved these changes Oct 9, 2024
@romain-intel
Copy link
Contributor

The issue though is that this won't load the files and they won't be included when moving to remote execution so I need to find another way; ie: fallback on iterating through the package the way I do for non-distribution packages.

And of course, the package could be fixed too :)

@trhodeos
Copy link
Contributor Author

trhodeos commented Oct 15, 2024

The issue though is that this won't load the files and they won't be included when moving to remote execution so I need to find another way; ie: fallback on iterating through the package the way I do for non-distribution packages.

And of course, the package could be fixed too :)

Yeah, that makes sense. For now, I've added an _ext_debug, with the [] so that metaflow doesn't crash and I've added a TODO.

I'll open an upstream patch for the extension so this metadata gets added as well!

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.

3 participants