Feat/allow parallel execution and pushes of ProjectRepos on two different machines#117
Conversation
hannahlanzrath
force-pushed
the
feat/allow_parallel_runs_of_projects
branch
13 times, most recently
from
bac99fe to
9044edd
Compare
|
Hey Hannah, thanks for putting this together. Just wondering, about the scope of this PR. Seems like there are some conflicts and some documentation related things which I thought we had already merged. Could you please check again if you were up to date on your base branch? |
hannahlanzrath
force-pushed
the
feat/allow_parallel_runs_of_projects
branch
2 times, most recently
from
fb51870 to
0eef9fd
Compare
Whoops, should be fine now again. |
hannahlanzrath
force-pushed
the
feat/allow_parallel_runs_of_projects
branch
2 times, most recently
from
430d71c to
61bbd5f
Compare
schmoelder
left a comment
schmoelder
left a comment
There was a problem hiding this comment.
As discussed earlier, this PR takes a strategy where we try avoiding conflicts in the first place. by first pulling in new data before committing. However, this only works if a single commit is added. Also I noticed that when running a reset --hard, it nukes all local changes.
Instead, we could go another route and try to rebase. I'm not sure how well it handles this but we should try.
cadetrdm/repositories.py
Outdated
|
|
||
| commit_obj = self.output_repo._git_repo.commit(output_branch_name) | ||
| output_repo_hash = commit_obj.hexsha | ||
| output_commit_message = commit_obj.message.replace("\n", "; ") |
There was a problem hiding this comment.
This is not really related to this PR but I don't really understand why we have a semicolon at the end of our commit messages...
There was a problem hiding this comment.
I have no clue honestly. :D
|
Thanks for the review! I see where I am introducing issues now, and I think I understand the problem better. I have another idea that might be much simpler and cleaner: Instead of appending to an existing |
hannahlanzrath
force-pushed
the
feat/allow_parallel_runs_of_projects
branch
4 times, most recently
from
6249f6a to
02bc99c
Compare
|
Okay so, with a fresh head I gave everything some thoughts again. And I have came to the conclusion that the test cases and issues that I thought about were more complex than the issue we actually have. So I have now just added rebase to our pull strategy when a conflict is hit. I tried it out on my machine and that worked with two different repos. :) So maybe you can give it a try again @schmoelder . |
schmoelder
left a comment
schmoelder
left a comment
There was a problem hiding this comment.
Nice work, I will test it later.
I agree, rebasing should work most of the time since we never edit the same line, and only add.
hannahlanzrath
force-pushed
the
feat/allow_parallel_runs_of_projects
branch
2 times, most recently
from
e87a8ca to
a412bed
Compare
|
I'm not done with testing yet, but if |
|
ok, seems to have worked! 🥳 |
hannahlanzrath
force-pushed
the
feat/allow_parallel_runs_of_projects
branch
from
a412bed to
44cff20
Compare
2 participants
Summary
This PR introduces a change to the output repository push logic to support parallel result pushes from multiple machines / instances of the from the same ProjectRepo plus corresponding tests and test changes.
The new test simulates two independent clones of the same CADET-RDM project pushing results to the same remote output repository.