Skip to content

Remove need for 'ISPYB_CREDENTIALS' environment variable#588

Merged
tieneupin merged 48 commits intomainfrom
remove-ispyb-creds-env-variable
May 9, 2025
Merged

Remove need for 'ISPYB_CREDENTIALS' environment variable#588
tieneupin merged 48 commits intomainfrom
remove-ispyb-creds-env-variable

Conversation

@tieneupin
Copy link
Contributor

@tieneupin tieneupin commented May 6, 2025

Resolves issue #587 .

ispyb.sqlalchemy.url() takes credentials as an argument, through which the config file can be passed directly to the function instead of relying on the ISPYB_CREDENTIALS environment variable. This eliminates the last traces of our repo's need to explicitly define ISPYB_CREDENTIALS.

Additionally, elements of the CI workflow and test code related to the Murfey and ISPyB databases have also been modified. ISPYB_CREDENTIALS has also been dropped successfully from the test workflow, and fixtures have been put in place to allow for the creation of databases that will be reset to a desired initial state at the end of every test. This means that the database does not have to be shut down and rebooted with every database-related test, which should help with our testing times as we continue to improve Murfey's test coverage.

NOTE: Within our code base, it looks like ISPyB spawns SQLAlchemy Session objects, whereas the Murfey database spawns SQLModel ones. As such, SQLModel's functions might not work when querying the ISPyB database. The database session fixtures for the ISPyB and Murfey test databases have been written to reflect this.

@tieneupin tieneupin requested a review from d-j-hatton May 6, 2025 08:39
@tieneupin tieneupin self-assigned this May 6, 2025
@tieneupin tieneupin added bug Something isn't working server Relates to the server component labels May 6, 2025
@codecov
Copy link

codecov bot commented May 6, 2025

Codecov Report

Attention: Patch coverage is 31.25000% with 22 lines in your changes missing coverage. Please review.

Project coverage is 30.17%. Comparing base (88a6f93) to head (f0f70f3).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #588      +/-   ##
==========================================
+ Coverage   30.15%   30.17%   +0.01%     
==========================================
  Files          80       80              
  Lines       10514    10517       +3     
  Branches     1405     1406       +1     
==========================================
+ Hits         3171     3174       +3     
+ Misses       7238     7237       -1     
- Partials      105      106       +1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tieneupin added 23 commits May 7, 2025 13:37
…; seed ISPyB database with ExperimentType information as well
@tieneupin tieneupin merged commit abcefc7 into main May 9, 2025
17 checks passed
@tieneupin tieneupin deleted the remove-ispyb-creds-env-variable branch May 9, 2025 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working server Relates to the server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ISPyB credentials still need to be passed in as an environment variable to load the ISPyB database correctly

2 participants