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

Script to generate URLs for Ultracool Sheet. Closes #389 #451

Merged
merged 8 commits into from
Jan 11, 2024

Conversation

kelle
Copy link
Collaborator

@kelle kelle commented Jan 8, 2024

Here is a script to generate the SIMPLE URL for sources in the Ultracool Sheet.

This draft tries to do URL checking but unfortunately, any invalid URLs give status code 200. We need to figure out how to do proper URL checking.

Copy link
Member

@Will-Cooper Will-Cooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can use urllib to handle strings

scripts/ultracool_sheet/generate_simple_links.py Outdated Show resolved Hide resolved
@kelle
Copy link
Collaborator Author

kelle commented Jan 9, 2024

Thanks for the suggestion!

Any thoughts on how to properly test the links? Right now, even bad links return 200 because you're doing good error handling on the website.

@Will-Cooper
Copy link
Member

Will-Cooper commented Jan 9, 2024

Hmm, how about

results: dict = db.inventory(simple_source)
assert('Sources' in results.keys())

or that sort of logic?
Because anything that passes that will have a website page. Or are we trying to specifically ascertain that the created url will be correct?

@Will-Cooper
Copy link
Member

Alternatively, I could edit the logic on the website to fail on a bad result and return a 404.

@kelle
Copy link
Collaborator Author

kelle commented Jan 9, 2024

Yeah, I really want to test the URL.

Maybe we give a 404 error but include all that useful text on the 404 page. Is that possible?

@Will-Cooper
Copy link
Member

Okie dokie I'll see what I can do

@Will-Cooper
Copy link
Member

Returning the exact text as would appear on the 404 page on the website isn't quite as simple and not really worthwhile (would have to make custom headers etc), because of how WSGI handles things server side. It's not really the done thing to override the default status code message.
But since that PR on the website, it will return a 404 code to an API request instead of 200.

@kelle kelle requested a review from Will-Cooper January 11, 2024 17:44
@kelle
Copy link
Collaborator Author

kelle commented Jan 11, 2024

Even though this doesn't modify the database, I think this script is worth having. I think we will end up building more scripts related to the sheet.

Copy link
Member

@Will-Cooper Will-Cooper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and I see from the email thread that you got a working csv of all matches!

@kelle kelle merged commit 735c7b7 into SIMPLE-AstroDB:main Jan 11, 2024
1 check passed
@kelle kelle deleted the sheet-match branch January 11, 2024 17:51
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.

2 participants