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: cannot import text_unidecode, nemo-toolkit #2068

Merged
merged 1 commit into from
Aug 2, 2024

Conversation

thundergolfer
Copy link
Contributor

@thundergolfer thundergolfer commented Aug 2, 2024

Describe your changes

Currently when Task tracing was turned on for a customer's modal.App the Task failed on startup due to errors in importlib machinery which do not occur normally.

The first issue was pypa/setuptools#4487. I could workaround that by doing setuptools==70.3.0 but then I hit other issues. Those other issues are fixed by the changeset here.

There's almost certainly more to understand about how an intercepting Loader should work but I think this change shouldn't introduce regressions.


Check these boxes or delete any item (or this section) if not relevant for this PR.

  • Client+Server: this change is compatible with old servers
    • only an internal telemetry change
  • Client forward compatibility: this change ensures client can accept data intended for later versions of itself
    • only an internal telemetry change

Copy link

@modal-pr-review-automation modal-pr-review-automation bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-approved 👍. This diff qualified for automatic approval and doesn't need follow up review.

Copy link
Contributor

@aksh-at aksh-at left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! Are there any other methods in the Loader interface we need to implement?

@thundergolfer
Copy link
Contributor Author

Are there any other methods in the Loader interface we need to implement?

This is somewhat answered by

There's almost certainly more to understand about how an intercepting Loader should work but I think this change shouldn't introduce regressions.

I would bet that there is, but understanding what needs to be implemented is a matter of grokking a graph of like a dozen abstract base classes that comprises the importlib system.

@thundergolfer thundergolfer merged commit 497c8e6 into main Aug 2, 2024
24 checks passed
@thundergolfer thundergolfer deleted the jonathon/telemetry-fix-aug2 branch August 2, 2024 21:57
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