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

Issue1157 add stat so subscribe/citypage downloads just work. #1163

Closed
wants to merge 23 commits into from

Conversation

petersilva
Copy link
Contributor

progress on #1157 ...

  • add a stat() entry point to the transfer protocols.
  • implemented using @reidsunderland 's and @andreleblanc11 's logic to fetch headers for https.
  • tested with subscribe/hpfx_amis

so before this change:

  • when downloading citypages, typically at the top of the hour, a subscriber would download a citypage and find it didn't match what the message said it should be.
  • so the downloaded file is discarded, and the message it placed on the retry queue.
  • five minutes later It will again attempt to download.

in the case of citypages, the file has been changed before the subscriber attempted the first download:

  • so all retries will fail.... for three days (by default retries continue for three days.)

with the change:

  • after a length mismatch on download, stat() the file on the server. to ask it what the size is on the server.

  • if the date of the file is newer than on the message, discard the message rather than requeing...

  • If the size matches on the server... then downloading again, won't change anything. so better to discard.

in both cases, subsequent retries will never succeed.,,, so just throw it out.

  • so there will be no error reported, the subscribe flow now understands what is going on and just prints and information message.
  • nothing will be put on the retry queue for this understood phenomenon.

So testing was done downloading subscribe/hpfx_citypage ... and with these changes, it is clean now, with no retries.

@petersilva
Copy link
Contributor Author

Also added fileSizeMax option to be able to avoid downloading files that are too much bigger than accepted.

Copy link

github-actions bot commented Aug 13, 2024

Test Results

1 tests   0 ✅  14s ⏱️
1 suites  0 💤
1 files    0 ❌  1 🔥

For more details on these errors, see this check.

Results for commit 8ba10cd.

♻️ This comment has been updated with latest results.

@petersilva
Copy link
Contributor Author

something weird has happenned to ubuntu 20.04... all tests on that OS are failing with some kind of SSL error.
I don't think it's related to this PR... on 22.04, all tests pass.

@petersilva petersilva marked this pull request as ready for review August 13, 2024 13:15
@petersilva
Copy link
Contributor Author

otoh, the problem seems to occur when running sr3 entry point and it does an import requests (which is added by this PR) it seems to fail with an inscrutable SSL something or other not found... so it's like the python packages being installed in 20.04 are somehow incompatible with eachother.

@petersilva
Copy link
Contributor Author

kind of frustrating... we have test machines, and one runs 18.04... and all the tests pass when I run them on a vm... tested so far on 24.04 and 18.04... hmm...

@petersilva
Copy link
Contributor Author

weirdnesses so far:

  • perhaps python3-requests is a new dependency that must be added (I think it is pulled in by other deps anyways... I did not have to install it, was there on my test machines.)
  • the metpx-sr3c packages might be a little janky on someOS's... it seems to be looking for the wrong versions in the flow tests... I built a local metpx-sr3c package to get it to pass on 18.04.

@reidsunderland
Copy link
Member

We could change the code to use urllib instead. Requests is just a little bit nicer to work with.

https://docs.python.org/3/library/urllib.request.html#urllib.request.Request --> can set method='HEAD'.

@petersilva
Copy link
Contributor Author

I'm going to close this PR, merge from development to the branch, and try opening the PR again.

@petersilva petersilva closed this Aug 16, 2024
andreleblanc11 and others added 2 commits August 16, 2024 11:42
See https://docs.python.org/3/library/socket.html#socket.timeout
socket.timeout has been replaced by TimeoutError.
It's only been converted to an alias of TimeoutError since python3.10

Co-authored-by: Reid Sunderland <[email protected]>
* implement messageAgeMax and post_ properly

* messageAgeMax documentation English

* French documentation of messageAgeMax settings

* clarifying message propoerties set by post_messageAgeMax for each message protocol
@petersilva petersilva mentioned this pull request Aug 16, 2024
@petersilva
Copy link
Contributor Author

re-submission behaves the same... it's something to do with the change... but no idea what.

@petersilva petersilva reopened this Aug 16, 2024
@petersilva
Copy link
Contributor Author

The failure on gitlab of images on 20.04 where the SSL crashes prevent running any tests... we are considering re-writing the https stat() call to use urllib.request()

@petersilva
Copy link
Contributor Author

This branch will require work and cleaning... will open a different PR at some point.

@petersilva petersilva closed this Aug 16, 2024
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.

3 participants