Repository navigation
Try to avoid "...//state.txt" URLs for replication. - #320
amandasaurus wants to merge 1 commit into
Conversation
9c83fb5 to
41267ea
Compare
lonvia
left a comment
lonvia
left a comment
There was a problem hiding this comment.
Fine with me in general. Unfortunately, merging is currently stuck on rather exotic CI fails of the Windows builds, which I have yet no idea how to fix.
| # Try to avoid calling `example.com/area-updates//state.txt` | ||
| baseurl_wo_slash = self.baseurl | ||
| while baseurl_wo_slash[-1] == "/": | ||
| baseurl_wo_slash = baseurl_wo_slash[:-1] |
There was a problem hiding this comment.
Probably better to do that in __init__ while assigning self.baseurl so it is only done once. (The variable could be overwritten later in theory but that would be unintended use, so safe to ignore.)
Also, you should use the endswith() function.
|
Looks like the Windows build system is keeping the build directory locked, which prevents it from being deleted during cleanup. Some leftover thread, possibly telemetry or other junk? I use cibuildwheel and have this: Fix: #325 |
41267ea to
30c274e
Compare
| (573923, 'https://text.org/000/573/923.state.txt'), | ||
| (3290012, 'https://text.org/003/290/012.state.txt'), | ||
| @pytest.mark.parametrize("baseurl,inp,outp", [ | ||
| ("https://text.org", None, 'https://text.org/state.txt'), |
There was a problem hiding this comment.
There is a tab here hidden somwehere which needs to go, see https://github.com/osmcode/pyosmium/actions/runs/38232342257/job/114754593730?pr=320
30c274e to
be011dc
Compare
Done.
Done. |
be011dc to
6b334e3
Compare
|
Do you mind rebasing this? CI should now run through again (except that the linter is still unhappy with your code ;) https://github.com/osmcode/pyosmium/actions/runs/38250018742/job/114813583694?pr=320) |
6b334e3 to
fd78ab7
Compare
Previously, for a base URL of "example.com/area-updates/" the code would look for "example.com/area-updates//state.txt" etc. This change will make it request "example.com/area-updates/state.txt". It now doesn't “double slash”. This will ensure providers of OSM data updates (e.g. Geofabrik) will see consistent URLs regardless of small typing changes.
fd78ab7 to
8ff9ea5
Compare
|
I've merged this now manually in 331784f with the suggested change and another fix for the tests. |
3 participants
Previously, for a base URL of
example.com/area-updates/the code would look forexample.com/area-updates//state.txt" etc. This change will make it requestexample.com/area-updates/state.txt`. It now doesn't “double slash”.This will ensure providers of OSM data updates (e.g. Geofabrik) will see consistent URLs regardless of small typing changes.
This change is inspired by Postel's Law: “be conservative in what you send, be liberal in what you accept”.