-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add google/latest endpoint #35
Conversation
print(arch) | ||
latest_image = ( | ||
db.query(GoogleImage) | ||
.filter(GoogleImage.arch.ilike(arch)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is interesting.
sqalchemy queries seem to be case sensitive by default (which is expected).
running a query like this will work:
"localhost:8000/google/latest?arch=ARM64"
but if I run this:
"localhost:8000/google/latest?arch=X86_64"
it will automatically be tranformed to "...x86_64" which will crash.
Used ilike comparison to mitigate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an interesting variance. 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few questions here.
cid/crud.py
Outdated
) | ||
print(archs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you intend to leave this print()
here? 😜
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope 🤦
cid/crud.py
Outdated
return {"error": "No images found", "code": 404} | ||
if latest_image is None: | ||
print("No image found") | ||
continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we return a 404 here?
21d78d4
to
e4d43c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but this one needs a rebase.
Add google/latest Add optional arch param Add tests to validate function with or without supplying opptional param
e4d43c2
to
9d9d002
Compare
Add google/latest
Add optional arch param
Add tests to validate function with or without supplying optional param