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

Fixed on_node failed build non-stopping problem. Now builds with on_n… #765

Merged
merged 4 commits into from
Apr 17, 2024

Conversation

dmageeLANL
Copy link
Collaborator

Fixes issue #763. Raises exception on non test.build_local build failure which stops pavilion from running the test anyway. And stops non test.build_local runs from writing BUILD_CREATED lines to status over and over when this test is queried.

There could probably be a better condition to stop writing BUILD_CREATED statuses. But in this case, if the run is completed or has an error status, it will keep that status.

Code review checklist:

  • Code is generally sensical and well commented
  • Variable/function names all telegraph their purpose and contents
  • Functions/classes have useful doc strings
  • Function arguments are typed
  • Added/modified unit tests to cover changes.
  • New features have documentation added to the docs.
  • New features and backwards compatibility breaks are noted in the RELEASE.md

@dmageeLANL dmageeLANL requested a review from Paul-Ferrell April 17, 2024 01:06
@dmageeLANL
Copy link
Collaborator Author

This PR now has a bonus fix! The original PR revealed a bug. In status_file.py:_parse_status_line, if the status timestamp is in the legacy format it can't be cast to a float and so the program skips into the first except. There it pops the status line list again, and tries to apply the legacy time format to that element. But that element is now the 2nd element which is the status state not the timestamp. So don't double pop, just reuse time_part.

Also, the failsafe call when = datetime.datetime(0, 0, 0) doesn't work either. If you don't want it to fail, use when = datetime.datetime.now().

This commit caught that because now status.current() is called in builder.py:TestBuilder:__init__(). And this is REALLY in the main line of the code. It's is called nearly every time the pav command is run. So when it hit the legacy test, it failed.

@Paul-Ferrell Paul-Ferrell merged commit 0496dc4 into hpc:master Apr 17, 2024
7 checks passed
hwikle-lanl pushed a commit that referenced this pull request Jun 7, 2024
#765)

* Fixed on_node failed build non-stopping problem. Now builds with on_node: true, do not run after the build failed and do not write BUILD_CREATED lines whenever queried.

* Add text to exception to satisfy style.

* Add particular exception to satisfy style.
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.

2 participants