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

Save observations and configuration statuses together #81

Merged
merged 2 commits into from Sep 16, 2019

Conversation

eheinrich
Copy link
Member

There was an error while serializing the response for a GET request to /api/schedule/. The error was a KeyError that was raised in the as_dict method of an Observation here, which suggests that a configuration status did not exist for one of the observation's configurations at the time of the method call.

The time of the error corresponds with the creation time of the observation in question- The observation was created at UT 9/8 05:34:00 and the timestamp in the error email is UT 9/8 05:34:05.

I've wrapped observation creation in a transaction so that an Observation will not be included in a queryset until both it and its config statuses have been saved. In the ScheduleSerializer save method, I included saving the request group in the transaction as well since I figure that if there is an error creating the observation, we probably don't want to keep the requestgroup around... Let me know what you think about that, especially given the work that you've been doing on your deadlock fixes branch.

@coveralls
Copy link

coveralls commented Sep 10, 2019

Coverage Status

Coverage increased (+0.001%) to 96.039% when pulling 8bd027d on fix/configstatus-race into 2401f3b on master.

Copy link
Member

@jnation3406 jnation3406 left a comment

Choose a reason for hiding this comment

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

yeah these look like good changes, they shouldn't interfere with my branch

@eheinrich eheinrich merged commit f7e0dd1 into master Sep 16, 2019
@eheinrich eheinrich deleted the fix/configstatus-race branch September 16, 2019 19:53
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.

None yet

3 participants