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.

Sign up for GitHub

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

Jump to bottom

Introduce extensions: Gubbins #249

Merged
merged 2 commits into from
Oct 15, 2024
Merged

Introduce extensions: Gubbins #249

merged 2 commits into from
Oct 15, 2024

Conversation

chaen
Copy link
Contributor

@chaen chaen commented Jun 4, 2024 •
edited
Loading

Closes #27

@chaen chaen mentioned this pull request Jun 4, 2024
Closed
@chaen chaen force-pushed the gubbins branch 2 times, most recently from 2578c3a to 1a1d20a Compare June 5, 2024 14:01
@chaen chaen force-pushed the gubbins branch 4 times, most recently from 171d908 to ef0b14e Compare September 21, 2024 14:39
@chaen chaen force-pushed the gubbins branch 5 times, most recently from e03a2f7 to 9b78e91 Compare October 1, 2024 10:17
@chaen chaen force-pushed the gubbins branch 2 times, most recently from fb89d8c to ba5cdc7 Compare October 14, 2024 14:23
@chaen chaen marked this pull request as ready for review October 14, 2024 14:23
@chaen
Copy link
Contributor Author

chaen commented Oct 14, 2024

@chrisburr @aldbr ready to review !

chrisburr
chrisburr reviewed Oct 15, 2024
View reviewed changes
extensions/README.md Outdated Show resolved Hide resolved
extensions/README.md Outdated Show resolved Hide resolved
extensions/README.md Outdated Show resolved Hide resolved
extensions/gubbins/gubbins-core/tests/test_config.py Outdated Show resolved Hide resolved
aldbr
aldbr reviewed Oct 15, 2024
View reviewed changes
extensions/README.md Outdated Show resolved Hide resolved
aldbr
aldbr reviewed Oct 15, 2024
View reviewed changes
extensions/README.md Outdated
A [router test](extensions/gubbins/gubbins-routers/tests/test_gubbins_job_manager.py) exists, even though no router is redefined. It is just to show that the correct DB is being loaded.

> [!WARNING]
> in the dependency override, you need to specify both the original DiracX JobDB as well as the extended one GubbinsJobDB. To avoid these issues, reuse the same name (i.e. `JobDB` instead of `GubbinsJobDB`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not crystal clear to me:

  • does "dependency override" mean <extension>-db/pyproject.toml?
  • why do I need and how can I specify both the original JobDB and the extended one?
  • "To avoid these issues", which ones? (Sorry I am reading that fast, so I might have overlooked something)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That here: https://github.com/chaen/diracx/blob/63e9f87d56d0f65da9669d4c33a3802483505b1f/extensions/gubbins/gubbins-routers/tests/test_gubbins_job_manager.py#L9-L23

I'll rephrase

aldbr
aldbr reviewed Oct 15, 2024
View reviewed changes
extensions/README.md Outdated Show resolved Hide resolved
aldbr
aldbr reviewed Oct 15, 2024
View reviewed changes
extensions/README.md Outdated Show resolved Hide resolved
aldbr
aldbr reviewed Oct 15, 2024
View reviewed changes
extensions/README.md Outdated Show resolved Hide resolved
@chrisburr chrisburr merged commit 83caaa0 into DIRACGrid:main Oct 15, 2024
24 checks passed
@chaen chaen mentioned this pull request Oct 17, 2024
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aldbr aldbr aldbr left review comments

@chrisburr chrisburr chrisburr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Example extension
4 participants
@chaen @chrisburr @aldbr @ruslan33