Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

Move SP config to yaml file #234

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Move SP config to yaml file #234

wants to merge 2 commits into from

Conversation

xinaxu
Copy link
Collaborator

@xinaxu xinaxu commented Dec 1, 2023

No description provided.

@xinaxu xinaxu requested a review from rvagg December 1, 2023 08:59
@rvagg
Copy link
Member

rvagg commented Dec 1, 2023

Nice, so do we still need the env file after this, can we move all the config to this so we don't have to describe two config files? And is singularity able to handle all of these config options on a per-SP basis or is there work to do on the singularity side coming out of this too?

@xinaxu
Copy link
Collaborator Author

xinaxu commented Dec 2, 2023

Nice, so do we still need the env file after this, can we move all the config to this so we don't have to describe two config files? And is singularity able to handle all of these config options on a per-SP basis or is there work to do on the singularity side coming out of this too?

We still need the env file. Lots of env vars are referenced in docker compose, e.g. db user/pass, singularity version, etc.

So I only split out settings related to SP to a config file which will be hot loaded.

@rvagg
Copy link
Member

rvagg commented Dec 2, 2023

Lots of env vars are referenced in docker compose

OK, but a lot of them are not too; maybe we just mark this as a future TODO item - move as much as possible into a config file, leaving the .env being only something we an ship as a default for users - items such as SINGULARITY_REF. A lot of these variables just end up being commandline arguments to motion itself, which we could easily ship in the same way as sps.yml.

So in light of that, maybe we could make it config.yml instead of sps.yml with an eye to evolving it into the single place that a user has to edit their stuff?

@xinaxu
Copy link
Collaborator Author

xinaxu commented Dec 6, 2023

Saving the current state of the work here.

@codecov-commenter
Copy link

Codecov Report

Merging #234 (f96e775) into main (421fde2) will increase coverage by 3.19%.
The diff coverage is 25.17%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #234      +/-   ##
==========================================
+ Coverage   44.00%   47.19%   +3.19%     
==========================================
  Files           9        9              
  Lines        1009      945      -64     
==========================================
+ Hits          444      446       +2     
+ Misses        513      444      -69     
- Partials       52       55       +3     
Files Coverage Δ
integration/singularity/options.go 34.72% <50.00%> (+11.25%) ⬆️
integration/singularity/store.go 36.63% <23.25%> (+0.75%) ⬆️

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants