-
Notifications
You must be signed in to change notification settings - Fork 112
Update item transactions in SQLAlchemy #303
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
Update item transactions in SQLAlchemy #303
Conversation
Change primary key of items table
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.
Added some comments.
Also this PR should have tests that prove the update and delete work in the case where two collections have the same Item ID, which I think would have caught the issues I pointed out.
stac_fastapi/sqlalchemy/stac_fastapi/sqlalchemy/transactions.py
Outdated
Show resolved
Hide resolved
I will write some tests |
@lossyrob Thanks for the feedback - I think the tests are good ... and everything is good now |
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.
I've gone through the changes here and they look good to me - @lossyrob were there any amendments you had in mind?
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, thanks @jonhealy1
Related Issue(s): #302
Description:
Different collections can have items with the same ids. To delete or update an item, a collection should be specified. In core.py for sqlalchemy, get_item was recently changed so that only an item from the right collection is returned. This is a similar idea.
PR Checklist:
pre-commit run --all-files
)make test
)make docs
)