Skip to content

Issue741 movedatabasecode #815

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
Closed
wants to merge 10 commits into from
Closed

Issue741 movedatabasecode #815

wants to merge 10 commits into from

Conversation

@HansVRP
Copy link
Contributor

@HansVRP HansVRP commented Oct 3, 2025

first proposal to seperate the job database/dataframe logic and the processbasedjobstart

@HansVRP HansVRP requested a review from soxofaan October 3, 2025 12:42
@HansVRP
Copy link
Contributor Author

HansVRP commented Oct 7, 2025

TODO; move also the unit tests once settled on rearangement

@soxofaan
Copy link
Member

soxofaan commented Oct 30, 2025

I'd try to do this exercise in atomic commits/PRs:

  • avoid moving unrelated things at the same time (like JobDatabaseInterface and ProcessBasedJobCreator here)
  • move related things in same commit: e.g. if you move ProcessBasedJobCreator, also move tests and update docs in same commit (or at least same PR)

that will avoid long-running PR review cycles that block other progress

e.g. I just started #824 to only move ProcessBasedJobCreator

@soxofaan soxofaan mentioned this pull request Oct 30, 2025
Merged
soxofaan
soxofaan reviewed Nov 3, 2025
View reviewed changes
Copy link
Member

@soxofaan soxofaan left a comment

Choose a reason for hiding this comment

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

some quick notes

@HansVRP HansVRP force-pushed the issue741_movedatabasecode branch from ed73677 to 57458a3 Compare November 4, 2025 12:24
@HansVRP HansVRP requested a review from soxofaan November 5, 2025 14:58
@soxofaan
ruff based code/import cleanup #741/#815
fa4f753
@soxofaan
Tighter encapsulation of "column requirements" of MultiBackendJobMana…
71971de 
…ger #741/#815

Using another solution to break the import cycle

(and some doc/test tweaks along the way)
soxofaan pushed a commit that referenced this pull request Nov 5, 2025
@HansVRP @soxofaan
Reorganize job_management submodule #741
c124c54 
This is a partial squash of #815
soxofaan added a commit that referenced this pull request Nov 5, 2025
@soxofaan
ruff based code/import cleanup #741/#815
308025a
soxofaan added a commit that referenced this pull request Nov 5, 2025
@soxofaan
Tighter encapsulation of "column requirements" of MultiBackendJobMana…
8a69ac3 
…ger #741/#815

Using another solution to break the import cycle

(and some doc/test tweaks along the way)
@soxofaan
Copy link
Member

soxofaan commented Nov 5, 2025

to not let this linger too long, I just did some follow-up commits, did a rebase and merged in 8bd7935

@soxofaan soxofaan closed this Nov 5, 2025
@soxofaan
Copy link
Member

soxofaan commented Nov 5, 2025

thanks for the refactor here!

@soxofaan soxofaan deleted the issue741_movedatabasecode branch November 5, 2025 19:00
@soxofaan soxofaan linked an issue Nov 5, 2025 that may be closed by this pull request
Job manager: Move jobdatabase classes out of __init__ #741
Closed
@soxofaan soxofaan mentioned this pull request Nov 5, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@soxofaan soxofaan Awaiting requested review from soxofaan

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Job manager: Move jobdatabase classes out of __init__

3 participants

@HansVRP @soxofaan