Fall back to _x filename for beta release notes #1455
Conversation
| settings.STATIC_CONTENT_BUCKET_NAME, | ||
| f"release-notes/master/{filename}.adoc", | ||
| ) | ||
| s3_client = get_s3_client() |
There was a problem hiding this comment.
I'd think we should tweak this so we have a set of alternative strings/formats checked in a while loop, that way there's flexibility for adding new ones in the future, and we only have the call code once.
There was a problem hiding this comment.
I considered this, but
- I wasn't really sure what other patterns might exist, making it a bit difficult to generalize, and
- YAGNI - I figured we could add this once we have more than two known possible patterns
If you think we should still add this now, I'm open to doing so, though we will still need a code change to at a minimum add the third pattern if it crops up. LMK your thoughts.
There was a problem hiding this comment.
Personally I'd have it not repeat the code in both places, but it will work as is so I've approved the PR.
There was a problem hiding this comment.
Fair - I have a feeling we'll be revisiting this code, and I'll keep this in mind. I'm going to add a TODO comment but merge this as-is for now. Thanks Dave!
| try: | ||
| response = session.get(url) | ||
| if response.status_code == 404: |
There was a problem hiding this comment.
same here, with the try catch inside the while
Follow-up fix for #1171
It turns out the newest beta release notes have been named
version_1_87_x.htmlinstead of the expectedversion_1_87_0.htmlname. This PR creates a fallback strategy that first attempts the_0filename, then tries_xif the former results in a 404 response.Manual testing
I wasn't able to successfully test the S3 version of the function, since the beta release notes do not yet appear to be in S3.
For the GitHub version, I added a print statement for debug purposes to indicate we were hitting the fallback url with the
_xfilename suffix.