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

Change to async SUMO calls #378

Merged
merged 1 commit into from
Oct 5, 2023
Merged

Change to async SUMO calls #378

merged 1 commit into from
Oct 5, 2023

Conversation

anders-kiaer
Copy link
Collaborator

@anders-kiaer anders-kiaer commented Sep 28, 2023

In progress. Currently blocked by equinor/fmu-sumo#224. Waiting on input on equinor/fmu-sumo#228.

Suggested pattern: Using a "factory method" (@classmethod) to initialize classes, and a parent-child-class-relationship which enables us to remove duplicated __init__ and from_case_uuid methods on access classes.

Other approaches tried:

  • Using __aenter__ and async context managers (since async code can not run in __init__), however this is more complex and would require multiple levels of idendation in cases different data type access classes are required in an endpoint.
  • Using @staticmethod. There is almost no difference to this approach compared with @classmethod. The benefit with the latter is that it is trivial, and the common Python pattern, to make factory methods on parent classes which can be used from child classes. With parent-child class relationship we can completely remove __init__ and the factory method on the child classes.
  • We have CaseInspector and IterationInspector, in addition to the different access classes.
    • There exists endpoints which uses instance of CaseInspector, IterationInspector and an access class. This results in three different async calls to sumo to verity three times the case uuid is unique. To easily avoid this, IterationInspector is renamed to SumoEnsemble (which is also the mentioned parent class for the other access classes). The CaseInspector is renamed to SumoCase and made the parent class to SumoEnsemble again, but this inheritence level is not as important I think so could be removed if we prefer.

Closes #240.

@anders-kiaer anders-kiaer self-assigned this Oct 1, 2023
@anders-kiaer anders-kiaer marked this pull request as ready for review October 2, 2023 11:09
@anders-kiaer anders-kiaer force-pushed the asyncsumo branch 2 times, most recently from c19d6ab to cce665b Compare October 5, 2023 12:15
@anders-kiaer anders-kiaer merged commit 1b0f6a4 into main Oct 5, 2023
6 checks passed
@anders-kiaer anders-kiaer deleted the asyncsumo branch October 5, 2023 19:02
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.

Go async on our FastAPI endpoints
2 participants