Run safety ensurer iso mds checkup on migrate events#2254
Run safety ensurer iso mds checkup on migrate events#2254JeffreyDevloo merged 12 commits into2.9.xfrom
Conversation
ghost
assigned
Shaved off some unittest time (2s)
Start on refactoring deduped mode
| if not registrations: | ||
| continue | ||
| # Filter out all the tasks are are no longer running within celery | ||
| running_registrations = [] |
There was a problem hiding this comment.
This improvement in naming <3
ovs/celery_run.py
Outdated
| if running_registrations == registrations: | ||
| # No changes required to be made | ||
| continue | ||
| persistent.assert_value(key, running_registrations, transaction=transaction) |
There was a problem hiding this comment.
You assert the value running_registrations, and immediately afterwards set it to this same value?
There was a problem hiding this comment.
Mistake on my end
| :param args: Arguments without default values | ||
| :param kwargs: Arguments with default values | ||
| """ | ||
| ensure_single = EnsureSingle(ensure_single_container, self) |
There was a problem hiding this comment.
This is always done in the inner functions, so why not do it in the general function?
There was a problem hiding this comment.
The self part is not defined in the general function
The constructor also fetches some runtime parameters which cannot be fetched on load time
@decorator is executed when loading in the module, this does not have any runtime info which the ensure single needs.
ovs/lib/helpers/decorators.py
Outdated
| # type: (dict, float, Tuple[str, str], int, int) -> None | ||
| """ | ||
| Ensure that the number of jobs don't exceed the limit | ||
| - Test if the current job limit is not reach |
ovs/lib/helpers/decorators.py
Outdated
| :type task_logs: Tuple[str, str] | ||
| :param job_limit: Number of jobs to keep running at any given time | ||
| :type job_limit: int | ||
| # @todo is this sane? Can't it be done with just the job limit? |
There was a problem hiding this comment.
Indeed, both cover the same use as far as i see
ovs/lib/helpers/decorators.py
Outdated
| try: | ||
| if slept: | ||
| self.logger.info('{0} had to wait {1} seconds before being able to start'.format(task_log_name, slept)) | ||
| if slept >= timeout: |
There was a problem hiding this comment.
will now log
X had to wait Y before being able to start Could not start Z`
upon timeout.
if slept:
slept < timeout:
log dis
else:
log dat
|
|
||
| successful = True | ||
| try: | ||
| if slept: |
There was a problem hiding this comment.
Same goes for this construction as previous comment
ovs/lib/helpers/decorators.py
Outdated
| :return: None | ||
| """ | ||
| if self.unittest_mode: | ||
| self.logger.info('Setting {} to {}'.format(self.thread_name, (state, value))) |
| else: | ||
| self.message = 'Ensure single {0} mode - ID {1} - {{0}}'.format(self.ensure_single_container.mode, self.now) | ||
|
|
||
| def unittest_set_state(self, state, value): |
There was a problem hiding this comment.
Perhaps set None as default for value, or does this have flaws?
| if not initial_registrations: | ||
| continue | ||
| # Filter out all the tasks are are no longer running within celery | ||
| # @todo might be better to do in update code? |
There was a problem hiding this comment.
Yup, please do this in the update code
There was a problem hiding this comment.
Once I backport it, I will
2 participants
No description provided.