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

Handle missing Neah Bay sea surface height obs by symlink to forecast file #114

Merged
merged 3 commits into from
Oct 19, 2022

Conversation

douglatornell
Copy link
Member

This moves handling of the exception case of persisting forecast values as observations when obs are missing/incomplete from upload_forcing to make_ssh_files. Doing so should avoid the symlink/file-exists race conditions that often arises when upload_forcing tries to create the symlink.

@douglatornell douglatornell added enhancement New feature or request Workers labels Oct 13, 2022
@douglatornell douglatornell added this to the v22.1 milestone Oct 13, 2022
@douglatornell douglatornell self-assigned this Oct 13, 2022
@codecov
Copy link

codecov bot commented Oct 13, 2022

Codecov Report

Merging #114 (071d3c7) into main (7066ba8) will decrease coverage by 0.36%.
The diff coverage is 89.47%.

@@            Coverage Diff             @@
##             main     #114      +/-   ##
==========================================
- Coverage   73.82%   73.45%   -0.37%     
==========================================
  Files         125      125              
  Lines       16278    16316      +38     
  Branches     1164     1168       +4     
==========================================
- Hits        12017    11985      -32     
- Misses       4206     4284      +78     
+ Partials       55       47       -8     
Flag Coverage Δ
unittests 73.45% <89.47%> (-0.37%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
tests/workers/test_make_ssh_file.py 83.21% <88.46%> (-16.79%) ⬇️
nowcast/workers/make_ssh_files.py 26.29% <91.66%> (-18.49%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@douglatornell
Copy link
Member Author

Worked as expected on 15oct22 for a day with complete obs (14oct) - this is normal operations confirmation.

obs -> fcts symlink was created as expected on 16oct22 when 15oct22 obs collected for forecast2 run were missing some values. Values were also missing later when the file for the forecast run was collected, so the symlink was not replaced with an obs file. This is full obs failure operations confirmation.

Not yet confirmed: operation when obs are missing at forecast2 time but available at forecast time. Vice versa is hypothetically possible too, I guess, but latter is a variant on the missing obs scenario that was confirmed on 16oct22.

Move code that confirms obs file existence or creates symlink to fcst file to
a function so that it can be tested, and the rest of the module test suite can
run without access to the `ssh dir` path.
@douglatornell douglatornell marked this pull request as ready for review October 18, 2022 23:11
@douglatornell douglatornell merged commit 9318982 into main Oct 19, 2022
@douglatornell douglatornell deleted the symlink-fcst-as-obs branch October 19, 2022 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Workers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant