Skip to content

Prevent to always create a run #814

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

Merged
merged 6 commits into from
Apr 15, 2025
Merged

Prevent to always create a run #814

merged 6 commits into from
Apr 15, 2025

Conversation

benoit-cty
Copy link
Contributor

@benoit-cty benoit-cty commented Mar 28, 2025

The ApiClient class is creating a run in init. But when we are using it only to get data it create useless entry in the database.

And now that the new API is in production, we could remove the warning.

@benoit-cty benoit-cty requested a review from inimaz March 28, 2025 20:33
@benoit-cty benoit-cty marked this pull request as ready for review April 2, 2025 16:47
Copy link
Collaborator

@inimaz inimaz left a comment

Choose a reason for hiding this comment

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

Thanks @benoit-cty! Maybe we can remove the call to self._create_run() in the __init__? I see that we create the run anyhow if not present in the add_emissions See here/

@benoit-cty
Copy link
Contributor Author

Thanks @benoit-cty! Maybe we can remove the call to self._create_run() in the __init__? I see that we create the run anyhow if not present in the add_emissions See here/

Well, I think not having a run_id in add_emissions is more a workaround in case of a network error.

I prefer the explicit way of not asking to add a run.

@benoit-cty benoit-cty requested a review from inimaz April 14, 2025 18:40
Copy link
Collaborator

@inimaz inimaz left a comment

Choose a reason for hiding this comment

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

Thanks @benoit-cty ! With this, the API client will be in better shape. I mentioned a minor comment

Copy link
Member

@SaboniAmine SaboniAmine left a comment

Choose a reason for hiding this comment

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

Thanks!

@benoit-cty benoit-cty merged commit a7d78ef into master Apr 15, 2025
4 checks passed
@benoit-cty benoit-cty deleted the feat/api_cleaning branch April 15, 2025 08:52
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