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

Fix test tracking #6701

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Conversation

berland
Copy link
Contributor

@berland berland commented Nov 28, 2023

Issue
Resolves #6694

Approach

Implement run_timeout_callback and run_exit_callback

@berland berland changed the base branch from main to python_jobqueue November 28, 2023 14:43
@berland berland self-assigned this Nov 28, 2023
@@ -347,6 +362,8 @@ async def execute(
await self._statechanges_to_publish.put(CLOSE_PUBLISHER_SENTINEL)
return EVTYPE_ENSEMBLE_CANCELLED

await asyncio.sleep(1)
Copy link
Contributor Author

@berland berland Nov 29, 2023

Choose a reason for hiding this comment

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

Moved here to allow async tasks to get done before we test for active-ness (slightly scary)

@berland berland marked this pull request as ready for review November 29, 2023 08:12
@berland berland enabled auto-merge (rebase) November 29, 2023 11:59
print(" done")

if state._callback_status_msg != "":
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this flipped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks strange but is copied directly from the existing job_queue_node, and it must be this way to pass tests ;)

return callback_status

async def run_timeout_callback(self, realization: RealizationState) -> None:
Copy link
Contributor

@xjules xjules Nov 29, 2023

Choose a reason for hiding this comment

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

We should unify calling this state and not realization I think; ie. run_timeout_callback(self, state: RealizationState)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

@xjules xjules left a comment

Choose a reason for hiding this comment

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

Got a few comments, but otherwise looks good!

Ensure storage state is set correctly when runs fail.

Adjust error string building

Passes test_tracking_integration
@berland
Copy link
Contributor Author

berland commented Nov 29, 2023

Repaired erroneous force push

Copy link
Contributor

@xjules xjules left a comment

Choose a reason for hiding this comment

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

Nice work! 🚀

@berland berland merged commit 070e8d5 into equinor:python_jobqueue Nov 29, 2023
29 of 40 checks passed
@berland berland deleted the fix_test_tracking branch May 29, 2024 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

test_tracking_integration - fix skipped tests
3 participants