Refactored the code consistent with the rest of the daemons#25
Open
omanikhi wants to merge 5 commits into
Open
Refactored the code consistent with the rest of the daemons#25omanikhi wants to merge 5 commits into
omanikhi wants to merge 5 commits into
Conversation
…e Daemon, DaemonTask, BackgroundJob, BackgroundWorker. - Use ArgumentParser instead of the outdated OptionParser - Rename JobMulti -> HistoryDumpJob, JobMultiLoad -> HistoryLoadJob - Fix pidfile write: os.open() returns an int, not a context manager; wrap with os.fdopen() - Fix flags/mode tuple bug: comma made them a tuple instead of separate arguments
- get_head now uses execute() from util instead of a manual svnlook|grep pipe; svnlook youngest outputs only the revision number so grep was redundant - Remove unused check_call wrapper and its assert (dead code)
heapq falls through to compare Job instances as a tiebreaker when rev is equal, which fails since BackgroundJob has no ordering. Add an itertools counter as a middle element in the priority tuple to prevent this.
- _get_svn_dump_args: was __ which caused a name-mangling AttributeError when called from HistoryDumpJob; changed to _ since subclasses need it - Align _get_key, _get_name, _validate_shard, _get_head, _get_shards to single underscore where subclasses access them, double underscore where the method is strictly internal to the defining class
Contributor
Author
|
@takesson Could you test it as well? I wrote a script and verified the fix for queue and also tested the --load and --history but I'm not sure I did it right. |
- tests/bgworkertest.py: test for BackgroundWorker.queue with same-rev jobs - pytest.ini: configures testpaths, *test.py pattern, and JUnit XML output - run-tests.sh: convenience script delegating to pytest - .gitignore: exclude generated test-reports/ directory
Member
|
I am not brave enough to merge this before CMS 5.3. Do we have a reference back to the issue / thread in Slack? |
Member
|
I have released 1.4.0 without this PR. |
Contributor
Author
Yes, I've now added it to the description. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Slack: https://simonsoft.slack.com/lists/T04L7JSEC/F07PTMY5G6P?record_id=Rec0AAV8A1QAE