Skip to content
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

Move config into data-models repo #320

Closed
adamjtaylor opened this issue Nov 15, 2023 · 13 comments · Fixed by #330
Closed

Move config into data-models repo #320

adamjtaylor opened this issue Nov 15, 2023 · 13 comments · Fixed by #330
Assignees
Labels
effort-low Should be a doddle

Comments

@adamjtaylor
Copy link
Contributor

We will move the config file looked at by DCA from the data_curator_config repo to the data-models repo.

@elv-sb and I discussed and agree that we will keep here under manual control and not implement its generation in CI.

@adamjtaylor
Copy link
Contributor Author

Slack discussion here.

https://sagebionetworks.slack.com/archives/C01ANC02U59/p1698873125444109

Need confirmation that DCA will look at a URL in the template_menu_config_file field of https://github.com/Sage-Bionetworks/data_curator_config/blob/main/dcc_config.csv

@aclayton555
Copy link
Contributor

From Anthony on 2023.12.01 - Not urgent - only the staging DCA has been migrated to the new config setup, so we can check this out. Prioritize this (and providing any feedback to FAIR) through December.

FAIR expecting to migrate configs and update prod in January, so any feedback we provide through December, can be taken into consideration in this process.

@aclayton555 aclayton555 added the effort-low Should be a doddle label Dec 4, 2023
@aclayton555
Copy link
Contributor

aclayton555 commented Dec 4, 2023

Actions:

  • move config to our repo and updating link to new config file in data curator config staging branch
  • perform testing (this sprint)
  • decide how this config is updated. Could leave this as static, with no CI to generate config. If adding a new component, have author update manually.
  • Longer term/TBD: Have separate issue for automating config generation via CI after JSON generation? Or, at least run a check to compare DIFF between JSON and config without pushing any changes

@adamjtaylor adamjtaylor linked a pull request Dec 6, 2023 that will close this issue
@adamjtaylor
Copy link
Contributor Author

Reopening while waiting on this PR to be merged: Sage-Bionetworks/data_curator_config#133

@adamjtaylor adamjtaylor reopened this Dec 6, 2023
@adamjtaylor
Copy link
Contributor Author

This is now deployed on the DCA staging instance. It seems to be working (I can select the expected templates in the drop downs)

@adamjtaylor
Copy link
Contributor Author

decide how this config is updated. Could leave this as static, with no CI to generate config. If adding a new component, have author update manually.

I think we agreed to leave static for now

@aclayton555
Copy link
Contributor

Discuss during 2024.01.04 mid-sprint - what, if any, action remaining here (and file tickets accordingly). From Anthony on 2024.01.02:

Hi everybody,
This Thursday, January 4th, DCA prod will switch to the new json config files. The main branch of data_curator_config will be changed to prod for consistency with DCA. No immediate action is required, though anyone contributing to their config files will need to use the prod branch. Let me know if you have any questions.

@adamjtaylor
Copy link
Contributor Author

@aclayton555
Copy link
Contributor

Confirm that there are no breaking changes and everything looks okay when we make our next release

@adamjtaylor
Copy link
Contributor Author

Waiting on Sage-Bionetworks/data_curator_config#139 for our latest version to be deployed with the new prod branch in the data curator config repo

@aclayton555
Copy link
Contributor

aclayton555 commented Jan 30, 2024

Remaining actions:

@aclayton555 aclayton555 reopened this Jan 30, 2024
@adamjtaylor
Copy link
Contributor Author

Waiting on Sage-Bionetworks/data_curator_config#147 to be merged to close this

@adamjtaylor
Copy link
Contributor Author

Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort-low Should be a doddle
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants