feat: File management for fetch-binary and fetch-science functions
#17
Conversation
BREAKING: some tests no longer pass
…e with `version=latest` argument
…oid issue with `version=latest` argument" This reverts commit bddeea8.
…e with `version=latest` argument
…oid issue with `version=latest` argument" This reverts commit bddeea8.
commit ee06eb9 Author: Michele Facchinelli <[email protected]> Date: Fri Aug 16 09:53:09 2024 +0000 fix: add packages permission commit d0bac86 Author: Michele Facchinelli <[email protected]> Date: Fri Aug 16 09:47:17 2024 +0000 fix: add checks permissions commit 80b3b92 Author: Michele Facchinelli <[email protected]> Date: Fri Aug 16 09:44:44 2024 +0000 fix: permissions for CI actions
|
@alastairtree, updated to version 0.9.0 of imap-data-access and tests successfully pass! |
|
|
||
| def upgrade() -> None: | ||
| # ### commands auto generated by Alembic - please adjust! ### | ||
| op.add_column("files", sa.Column("version", sa.Integer(), nullable=False)) |
There was a problem hiding this comment.
This upgrade will probably fail if the database contains any existing rows because you cannot add a non-nullable column without a default value to an existing row. Fine for this since we do not care about the data but just flagging this will need us to drop and recreate the data when we deploy this. A good practise is to always provide a default value when adding columns to existing tables, or to add as nullable, populate the data and then update it and make it non nullable.
There was a problem hiding this comment.
That makes sense. I did not consider that!
Is this any better?
There was a problem hiding this comment.
(sorry - will push changes soon!)
| docker = "^7.1.0" | ||
| testcontainers = "^4.7.2" | ||
| wiremock = {git = "https://github.com/ImperialCollegeLondon/python-wiremock.git", rev = "fix-test-containers-on-windows"} |
There was a problem hiding this comment.
I just chased on the wiremock Slack to try and get this PR bugfix we need merged but no luck so far.
| @@ -40,3 +68,61 @@ def insert_files(self, files: list[File]): | |||
| raise e | |||
| finally: | |||
| session.close() | |||
|
|
|||
|
|
|||
| class DatabaseOutputManager(IOutputManager): | |||
There was a problem hiding this comment.
I don't really like the name "output manager" because i can't really tell what it is but not sure if there is anything better. Basically just talking to myself here.
Adds
OutputManagerclass to manage output of files to requestedoutputfolder and (optionally) database. Currently only supported forfetch-scienceandfetch-binarymethods.Plenty of unit tests for new classes. See new coverage report in GitHub Actions.