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

ORM: Fix problem with detached DbAuthInfo instances #6208

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Dec 8, 2023

Fixes #4596

The idea is that somehow, the AuthInfo instance that is kept as a reference by the JobsList goes stale at some point when a CalcJob requests a transport through the JobManager. The fix is to use the explicit AuthInfo that is passed when request_job_info_update is called and not the one that was used to construct the original JobsList instance.

@sphuber sphuber force-pushed the fix/4596/db-authinfo-detached branch from 001f24f to f237fdf Compare December 8, 2023 21:33
@sphuber sphuber requested a review from unkcpz December 13, 2023 10:06
@unkcpz
Copy link
Member

unkcpz commented Dec 14, 2023

Thanks! The changes look good.
If I understand the behavior of the JobManager correctly, I guess after the job is submitted, the change to the minimum_job_interval will take effect on the job immediately without restarting the daemon?

@sphuber
Copy link
Contributor Author

sphuber commented Dec 14, 2023

Thanks! The changes look good. If I understand the behavior of the JobManager correctly, I guess after the job is submitted, the change to the minimum_job_interval will take effect on the job immediately without restarting the daemon?

I think so yes. In aiida.engine.process.calcjobs.tasks.task_update_job the TaskManager.request_job_info_update method is called, passing the AuthInfo that is retrieved from node.get_authinfo(). But it could be that that last call is using a cached instance of AuthInfo. Then again, that should be wrapped in our ModelWrapper which ensures that fetching attributes should cause the model to be updated from the database, if the model is stored. So long story short, yes, I think this should automatically update the polling interval without restarting the daemon 😄

Copy link
Member

@unkcpz unkcpz left a comment

Choose a reason for hiding this comment

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

So long story short, yes, I think this should automatically update the polling interval without restarting the daemon 😄

This is nice!

The idea is that somehow, the `AuthInfo` instance that is kept as a
reference by the `JobsList` goes stale at some point when a `CalcJob`
requests a transport through the `JobManager`. The fix is to use the
explicit `AuthInfo` that is passed when `request_job_info_update` is
called and not the one that was used to construct the original
`JobsList` instance.
@sphuber sphuber force-pushed the fix/4596/db-authinfo-detached branch from f237fdf to c44a92c Compare December 14, 2023 09:57
@sphuber sphuber merged commit ec2c6a8 into aiidateam:main Dec 14, 2023
19 checks passed
@sphuber sphuber deleted the fix/4596/db-authinfo-detached branch December 14, 2023 10:23
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.

Error in SQLA (DetachedInstanceError) (in AiiDA v1.5 and newly reported in AiiDA v2.0.1)
2 participants