-
Notifications
You must be signed in to change notification settings - Fork 107
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
Remove ert-storage as dependency #6559
Conversation
4da99e0
to
9c6a94d
Compare
5b78713
to
d7b56ca
Compare
Should we also use the opportunity to rename |
src/ert/services/_storage_main.py
Outdated
# Dark Storage imports from ERT Storage, which connects to the database | ||
# at startup. We set the database URL to an SQLite in-memory database so | ||
# that the import succeeds. | ||
os.environ["ERT_STORAGE_DATABASE_URL"] = "sqlite://" |
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.
Think we can remove the DB references. There are also some references to sqlite
around the code
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 agree. Dark storage doesn't use a traditional RDBMS.
This particular line is a hack to get ert-storage to initialise, as the database connection is made during import. Particularly, we needed to determine whether the we are using SQLite or Postgresql, and modify the schema accordingly. Because Dark Storage imports from ert-storage
, it'd import the database schema which requires the ERT_STORAGE_DATABASE_URL
to be set. We set it to sqlite://
here, which specifies to create an in-memory sqlite database.
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.
Please also put a link to the github repo and the commit from which you took this code, so that the git history is at least somewhat preserved.
src/ert/services/_storage_main.py
Outdated
# Dark Storage imports from ERT Storage, which connects to the database | ||
# at startup. We set the database URL to an SQLite in-memory database so | ||
# that the import succeeds. | ||
os.environ["ERT_STORAGE_DATABASE_URL"] = "sqlite://" |
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 agree. Dark storage doesn't use a traditional RDBMS.
This particular line is a hack to get ert-storage to initialise, as the database connection is made during import. Particularly, we needed to determine whether the we are using SQLite or Postgresql, and modify the schema accordingly. Because Dark Storage imports from ert-storage
, it'd import the database schema which requires the ERT_STORAGE_DATABASE_URL
to be set. We set it to sqlite://
here, which specifies to create an in-memory sqlite database.
I would like to at least add |
I'd rather we do that in a separate PR I think. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #6559 +/- ##
========================================
Coverage 83.57% 83.57%
========================================
Files 345 359 +14
Lines 20761 21018 +257
Branches 948 948
========================================
+ Hits 17350 17565 +215
- Misses 3117 3159 +42
Partials 294 294 ☔ View full report in Codecov by Sentry. |
Code taken from: equinor/ert-storage@7137f62
I've squashed and added a link to the latest commit from ert-storage in the commit message body. |
Pre review checklist
Ground Rules),
and changes to existing code have good test coverage.
Pre merge checklist