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

hydrate functions then classes in container entrypoint #2583

Merged
merged 2 commits into from
Nov 28, 2024

Conversation

devennavani
Copy link
Contributor

@devennavani devennavani commented Nov 28, 2024

Describe your changes

#2364 introduced a subtle requirement that hydrating a class requires that the class service function for the class is hydrated first.

in _run_app we make sure to resolver.load an object's deps before loading the object itself, but in container entrypoint we are hydrating a class before we hydrate the class service function, so the _method_functions of the Cls was empty because the class service function was unhydrated (its method_handle_metadata attribute was not populated).

this PR changes the entrypoint to load the app's functions, then its classes

@devennavani devennavani marked this pull request as ready for review November 28, 2024 03:56
@devennavani
Copy link
Contributor Author

@prbot approve

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.

Approved 👍. @mwaskom will follow-up review this.

@devennavani devennavani merged commit bd320d8 into main Nov 28, 2024
20 checks passed
@devennavani devennavani deleted the deven/hydrate_functions_then_classes branch November 28, 2024 04:12
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.

1 participant