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

Check in smoketest urls #8433

Merged
merged 3 commits into from
May 24, 2024
Merged

Check in smoketest urls #8433

merged 3 commits into from
May 24, 2024

Conversation

wpears
Copy link
Member

@wpears wpears commented May 21, 2024

#8429 didn't take because, currently, the true list of urls to test lives in S3. I believe the logic here was that it was easier to update and/or would hook into a dynamically updating list.

These things don't actually happen, so I believe it's actually better practice to switch the fallback url_list to the actual list. This makes the script more obvious in behavior (it was already obvious if I actually read the code instead of just noticing the list... but now it's REALLY obvious) and feels a little more deliberate/less magic.

@wpears wpears requested review from willbarton and chosak May 21, 2024 14:20
@chosak
Copy link
Member

chosak commented May 21, 2024

What are the changes in cfgov/cfgov/urls.py? Are those unrelated?

@wpears
Copy link
Member Author

wpears commented May 21, 2024

What are the changes in cfgov/cfgov/urls.py? Are those unrelated?

Those are redirects to redirects that I noticed when running the script locally. Just shortening the redirect chain here.

@wpears wpears added this pull request to the merge queue May 24, 2024
Merged via the queue into main with commit 196d528 May 24, 2024
12 checks passed
@wpears wpears deleted the check-in-smoketest-urls branch May 24, 2024 13:04
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.

None yet

3 participants