-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: File management for fetch-binary
and fetch-science
functions
#17
Open
mfacchinelli
wants to merge
104
commits into
main
Choose a base branch
from
feat/hk_manage
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,359
−251
Open
Changes from all commits
Commits
Show all changes
104 commits
Select commit
Hold shift + click to select a range
fe26c60
feat: add class to manage outputs
mfacchinelli 231e0c2
refactor: version is an `int` not a `str`
mfacchinelli 3741a1f
refactor: replace custom logic with `OutputManager`
mfacchinelli fdcd9ba
refactor: convert "pattern" methods to separate "provider" class
mfacchinelli 8ca6ca3
build: single source app version
mfacchinelli b4087a2
style: replace `-` with `_` in file names
mfacchinelli da4c44b
task: add support for `prefix` and `level` in output metadata
mfacchinelli 99773b5
feat: add more variables in database
mfacchinelli 7ede655
feat!: use output manager to write to output and database
mfacchinelli 76afe1e
dev: add `SQLALCHEMY_URL` to dev container environment
mfacchinelli f6927b1
test(fix): fix output manager and calibration tests
mfacchinelli 3eb23f1
fix: do not export to database in tests
mfacchinelli 332cb24
fix: HK and science tests
mfacchinelli de14a6f
fix: attempt to fix issue with database initialization
mfacchinelli fdb5882
build: add `pytest-mock` to dev dependencies
mfacchinelli 8cbe6be
test: add tests for `FetchBinary` class
mfacchinelli 527bcc6
test: add unit tests for `FetchScience`
mfacchinelli bc374d3
chore: update lock file
mfacchinelli c7c274f
task: add `hash` to database and make `path` a unique constraint
mfacchinelli e95ac5f
feat: update database handler to check for error cases
mfacchinelli 87dce32
fix: remove logging of URL
mfacchinelli 050d8b4
test: move `create_test_file` to utility and use `assert_not_called` …
mfacchinelli fa82965
test: add some tests for database output manager
mfacchinelli 84770f3
test: add coverage for database error
mfacchinelli 3ae9fc4
Merge remote-tracking branch 'origin/main' into feat/hk_manage
mfacchinelli 739765b
build: change `imap-mag` version to initial version
mfacchinelli 94831b2
test(fix): remove commented out text
mfacchinelli bddeea8
fix: temporarely use custom version of imap-data-access to avoid issu…
mfacchinelli 3239efb
Revert "fix: temporarely use custom version of imap-data-access to av…
mfacchinelli 164b61b
build: add coverage report to CI action
mfacchinelli 7c9ebd4
test: skip failing test due to bug in `imap-data-access`
mfacchinelli 9145268
build(fix): use version instead of branch
mfacchinelli 67b1726
test(fix): name of classes under test in comment
mfacchinelli 9540787
test: add more tests to increase coverage
mfacchinelli 39a30a6
test(fix): actually use `convertToDatetime`
mfacchinelli 65db0ff
fix: CI action failure, packaging failure, test failure
mfacchinelli afa22f2
fix: more fixes to permissions for action and packaging
mfacchinelli cffe685
Squashed commit of the following:
mfacchinelli f342eb1
task: fix src paths and docker multistage builds
alastairtree dc72096
task: tidy dockerfile
alastairtree 4857812
Initial add of CI step to test on Windows
mhairifin 43d8083
install patched wiremock and fix Path usage
mhairifin 65ebba6
clone from https rather than ssh
mhairifin 8dbd7da
fix: add yaml representer for a indowsPath
mhairifin 455277f
fix: remove hashes from requirements so git package can be installed
mhairifin 4e27257
install git in dockerfile
mhairifin 3a40b76
Correct formatting
mhairifin 39df053
update dependency on patched python wiremock to dev dependency
mhairifin df730b8
Update wiremock to patch it so no path manipulations necessary
mhairifin 8b98e37
Correct code formatting
mhairifin d71e359
Skip tests that will fail on Windows runner
mhairifin e0eee44
Fix ruff errors
mhairifin 4f922b8
Fix ruff errors
mhairifin 4e8d20e
Running pre-commit
mhairifin 1163a78
chore: fix line ending in pre-commit
alastairtree 1739fe6
feat: add class to manage outputs
mfacchinelli fc771c3
refactor: version is an `int` not a `str`
mfacchinelli d1099d9
refactor: replace custom logic with `OutputManager`
mfacchinelli 4d66bed
refactor: convert "pattern" methods to separate "provider" class
mfacchinelli 620da38
build: single source app version
mfacchinelli 1a75c22
style: replace `-` with `_` in file names
mfacchinelli cb4186c
task: add support for `prefix` and `level` in output metadata
mfacchinelli cbe7e83
feat: add more variables in database
mfacchinelli a69dd9b
feat!: use output manager to write to output and database
mfacchinelli 5093519
dev: add `SQLALCHEMY_URL` to dev container environment
mfacchinelli 5510d3a
test(fix): fix output manager and calibration tests
mfacchinelli 28ce147
fix: do not export to database in tests
mfacchinelli 9b353bc
fix: HK and science tests
mfacchinelli d780de8
fix: attempt to fix issue with database initialization
mfacchinelli f58a6a7
build: add `pytest-mock` to dev dependencies
mfacchinelli f7e9fd1
test: add tests for `FetchBinary` class
mfacchinelli 1a6178a
test: add unit tests for `FetchScience`
mfacchinelli b8737aa
chore: update lock file
mfacchinelli 8a9236d
task: add `hash` to database and make `path` a unique constraint
mfacchinelli 56d8fc1
feat: update database handler to check for error cases
mfacchinelli a366cbc
fix: remove logging of URL
mfacchinelli 97ff8f2
test: move `create_test_file` to utility and use `assert_not_called` …
mfacchinelli c22489b
test: add some tests for database output manager
mfacchinelli 5dbf6dd
test: add coverage for database error
mfacchinelli 490a515
build: change `imap-mag` version to initial version
mfacchinelli d4ac011
test(fix): remove commented out text
mfacchinelli e4b545d
fix: temporarely use custom version of imap-data-access to avoid issu…
mfacchinelli 7114e0c
Revert "fix: temporarely use custom version of imap-data-access to av…
mfacchinelli f786a87
build: add coverage report to CI action
mfacchinelli e9694c9
test: skip failing test due to bug in `imap-data-access`
mfacchinelli 3a82f36
build(fix): use version instead of branch
mfacchinelli 6c368f4
test(fix): name of classes under test in comment
mfacchinelli 01360fb
test: add more tests to increase coverage
mfacchinelli 0a0a455
test(fix): actually use `convertToDatetime`
mfacchinelli 91f0c34
fix: CI action failure, packaging failure, test failure
mfacchinelli ea59ccb
fix: more fixes to permissions for action and packaging
mfacchinelli 693cd49
Squashed commit of the following:
mfacchinelli 921768a
Merge branch 'feat/hk_manage' of https://github.com/ImperialCollegeLo…
mfacchinelli 9c86b45
fix: merge issues
mfacchinelli 36d8ecf
build(fix): use existing permissions
mfacchinelli 6218f99
fix: install git on Docker image
mfacchinelli 41decd0
try: use git under wine
mfacchinelli 844213b
try: force Docker run action to finish
mfacchinelli a041109
try: more tests to make git work on Docker
mfacchinelli 37fa416
fix: give up on git-based imap-data-access and skip failing test
mfacchinelli 1bcb16a
fix: update to latest `imap-data-access` and unfilter test
mfacchinelli 55e0838
fix: partially address Alastair's comments
mfacchinelli 49a8d22
fix: avoid circular dependency
mfacchinelli 003cf5a
fix: further reduce duplication with hash
mfacchinelli File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -135,3 +135,4 @@ site/ | |
.work | ||
/output | ||
dev.env | ||
debug |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
52 changes: 52 additions & 0 deletions
52
...p_db/migrations/versions/2024_08_09-669111c45c37_added_version_hash_date_and_software_.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
"""Added version, hash, date and software version columns | ||
|
||
Revision ID: 669111c45c37 | ||
Revises: d0457f3e98c8 | ||
Create Date: 2024-08-09 13:35:21.578940 | ||
|
||
""" | ||
|
||
from datetime import datetime | ||
|
||
import sqlalchemy as sa | ||
from alembic import op | ||
|
||
# revision identifiers, used by Alembic. | ||
revision = "669111c45c37" | ||
down_revision = "d0457f3e98c8" | ||
branch_labels = None | ||
depends_on = None | ||
|
||
|
||
def upgrade() -> None: | ||
# ### commands auto generated by Alembic - please adjust! ### | ||
op.add_column( | ||
"files", sa.Column("version", sa.Integer(), nullable=False, default=0) | ||
) | ||
op.add_column( | ||
"files", sa.Column("hash", sa.String(length=64), nullable=False, default="") | ||
) | ||
op.add_column( | ||
"files", | ||
sa.Column( | ||
"date", sa.DateTime(), nullable=False, default=datetime.fromtimestamp(0) | ||
), | ||
) | ||
op.add_column( | ||
"files", | ||
sa.Column( | ||
"software_version", sa.String(length=16), nullable=False, default="0.0.0" | ||
), | ||
) | ||
op.create_unique_constraint(None, "files", ["path"]) | ||
# ### end Alembic commands ### | ||
|
||
|
||
def downgrade() -> None: | ||
# ### commands auto generated by Alembic - please adjust! ### | ||
op.drop_constraint(None, "files", type_="unique") | ||
op.drop_column("files", "software_version") | ||
op.drop_column("files", "date") | ||
op.drop_column("files", "hash") | ||
op.drop_column("files", "version") | ||
# ### end Alembic commands ### |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,34 @@ | ||
import abc | ||
import logging | ||
import os | ||
from pathlib import Path | ||
|
||
import typer | ||
from imap_db.model import File | ||
from sqlalchemy import create_engine | ||
from sqlalchemy.orm import sessionmaker | ||
|
||
from imap_mag import __version__ | ||
from imap_mag.outputManager import IFileMetadataProvider, IOutputManager, generate_hash | ||
|
||
|
||
class IDatabase(abc.ABC): | ||
"""Interface for database manager.""" | ||
|
||
def insert_file(self, file: File) -> None: | ||
"""Insert a file into the database.""" | ||
self.insert_files([file]) | ||
pass | ||
|
||
@abc.abstractmethod | ||
def insert_files(self, files: list[File]) -> None: | ||
"""Insert a list of files into the database.""" | ||
pass | ||
|
||
|
||
class Database(IDatabase): | ||
"""Database manager.""" | ||
|
||
class DB: | ||
def __init__(self, db_url=None): | ||
env_url = os.getenv("SQLALCHEMY_URL") | ||
if db_url is None and env_url is not None: | ||
|
@@ -16,10 +39,12 @@ def __init__(self, db_url=None): | |
"No database URL provided. Consider setting SQLALCHEMY_URL environment variable." | ||
) | ||
|
||
# TODO: Check database is available | ||
|
||
self.engine = create_engine(db_url) | ||
self.Session = sessionmaker(bind=self.engine) | ||
|
||
def insert_files(self, files: list[File]): | ||
def insert_files(self, files: list[File]) -> None: | ||
session = self.Session() | ||
try: | ||
for file in files: | ||
|
@@ -40,3 +65,60 @@ def insert_files(self, files: list[File]): | |
raise e | ||
finally: | ||
session.close() | ||
|
||
|
||
class DatabaseOutputManager(IOutputManager): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
"""Decorator for adding files to database as well as output.""" | ||
|
||
__output_manager: IOutputManager | ||
__database: IDatabase | ||
|
||
def __init__( | ||
self, output_manager: IOutputManager, database: Database | None = None | ||
): | ||
"""Initialize database and output manager.""" | ||
|
||
self.__output_manager = output_manager | ||
|
||
if database is None: | ||
self.__database = Database() | ||
else: | ||
self.__database = database | ||
|
||
def add_file( | ||
self, original_file: Path, metadata_provider: IFileMetadataProvider | ||
) -> tuple[Path, IFileMetadataProvider]: | ||
(destination_file, metadata_provider) = self.__output_manager.add_file( | ||
original_file, metadata_provider | ||
) | ||
|
||
file_hash: str = generate_hash(original_file) | ||
|
||
if not ( | ||
destination_file.exists() and (generate_hash(destination_file) == file_hash) | ||
): | ||
logging.error( | ||
f"File {destination_file} does not exist or is not the same as original {original_file}." | ||
) | ||
destination_file.unlink(missing_ok=True) | ||
raise typer.Abort() | ||
|
||
logging.info(f"Inserting {destination_file} into database.") | ||
|
||
try: | ||
self.__database.insert_file( | ||
File( | ||
name=destination_file.name, | ||
path=destination_file.absolute().as_posix(), | ||
version=metadata_provider.version, | ||
hash=file_hash, | ||
date=metadata_provider.date, | ||
software_version=__version__, | ||
) | ||
) | ||
except Exception as e: | ||
logging.error(f"Error inserting {destination_file} into database: {e}") | ||
destination_file.unlink() | ||
raise e | ||
|
||
return (destination_file, metadata_provider) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
"""The main module for project.""" | ||
|
||
from importlib.metadata import PackageNotFoundError, version | ||
|
||
|
||
def get_version() -> str: | ||
try: | ||
return version("imap-mag") | ||
except PackageNotFoundError: | ||
print("IMAP MAG CLI Version unknown, not installed via pip.") | ||
|
||
|
||
__version__ = get_version() |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 just chased on the wiremock Slack to try and get this PR bugfix we need merged but no luck so far.