-
Notifications
You must be signed in to change notification settings - Fork 1
Enhancements/clob #73
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
base: main
Are you sure you want to change the base?
Enhancements/clob #73
Conversation
Cleanup BLOB support to match CLOB implementation. list command has left-justified output for easier reading.
6504e47 to
371859e
Compare
krowvin
left a comment
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.
Thank you for the type fixes/removing unused methods in blobs!
Few questions and one suggested fix before merging.
Fix CI/CD
@Enovotny approved but it does look like black was not setup locally? Admittedly I don't think we have a contributing yet but you can see how to setup black locally with Poetry here:
https://github.com/HydrologicEngineeringCenter/cwms-python/blob/main/CONTRIBUTING.md#getting-started
That will fix formatting issues on commit to match the repo standard setup by Eric
Ignoring the formatting now could end up making for confusing commits later, imo
| logging.info(f"Uploaded blob: {blob_id_up}") | ||
| logging.info(f"View: {api_root}blobs/{blob_id_up}?office={office}") | ||
| if has_invalid_chars(blob_id_up): | ||
| logging.info(f"View: {api_root}blobs/ignored?blob-id={blob_id_up}&office={office}") |
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.
blobs/ignored? Is this a hidden endpoint to allow for blob id in a URI with a path?
If this requires your fix to CDA (i.e. it's not a hidden endpoint now) we might need to lock the package to a specific CDA version and call the version endpoint to test the version on cwms-cli first run. Instead of assuming the /ignored endpoint exists in a given instance.
Think delayed adoption type stuff. And the /v2 /v3 etc does not exist in CDA yet.
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.
'ignored' is recommended in the CDA documentation, see /blobs/{blob-id} endpoint.
| return dest | ||
|
|
||
|
|
||
| def store_blob(**kwargs): |
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.
Thanks for removing this! I missed it
|
Drafting this until the cwms-python PR is merged |
Implements #72
Add CLOB support like BLOB.
Dependencies: USACE/cwms-data-api#1483 HydrologicEngineeringCenter/cwms-python#242