Simplifications for DependencyManager #13533
Conversation
|
Surprisingly, this passes all the integration tests. |
awx/main/scheduler/task_manager.py
Outdated
| @@ -323,21 +303,13 @@ def should_update_inventory_source(self, job, latest_inventory_update): | |||
| return False | |||
|
|
|||
| timeout_seconds = timedelta(seconds=latest_inventory_update.inventory_source.update_cache_timeout) | |||
| if (latest_inventory_update.finished + timeout_seconds) < now: | |||
| if (latest_inventory_update.finished + timeout_seconds) < job.created: | |||
There was a problem hiding this comment.
This is not good, need to change this or otherwise think about more.
awx/main/scheduler/task_manager.py
Outdated
| @@ -393,7 +354,7 @@ def gen_dep_for_job(self, task): | |||
| continue | |||
| if not inventory_source.update_on_launch: | |||
| continue | |||
| latest_inventory_update = self.get_latest_inventory_update(inventory_source) | |||
| latest_inventory_update = InventoryUpdate.objects.filter(inventory_source=inventory_source).order_by("-created").first() | |||
There was a problem hiding this comment.
probably want inventory_source.inventory_updates
AlanCoding
force-pushed
the
dep_simple
branch
from
503c563
to
df77d1d
Compare
awx/main/scheduler/task_manager.py
Outdated
| for task in undeped_tasks: | ||
| if task.dependencies_processed: | ||
| continue | ||
| processed_ids.append(task.id) |
There was a problem hiding this comment.
@fosterseth I added this because we call generate_dependencies an extra time for in case of deps-of-deps, passing in deps. In most cases those deps do not need to have their own deps generated. I will trust my optimizations to create_unified_job, and if a dependent project update has dependencies_processed as False then obviously we would like to exclude it from this loop for performance and to avoid errors from re-processing.
AlanCoding
changed the title
| If it has never updated, we need to update | ||
| If there is already an update in progress then we do not need to a new create one | ||
| If the last update failed, we always need to try and update again | ||
| If current time is more than cache_timeout after last update, then we need a new one | ||
| ''' |
There was a problem hiding this comment.
You can summarize the diff around here by saying that we combined 2 methods into 1. That is:
should_update_inventory_source & should_update_related_project --> should_update_again
the method signature changed slightly from (self, job, latest_project_update/latest_inventory_update) to (update, cache_timeout). Maybe I should call it latest_update instead of update for clarity, and the fact that it's the same as the last parameter of the old method. This can be either a project update or a inventory update. And now that I think about it, it might be better to infer cache_timeout inside of this method, getting us down to 1 argument, because that is truly the only thing we need.
AlanCoding
force-pushed
the
dep_simple
branch
from
d5db609
to
f04acc5
Compare
AlanCoding
force-pushed
the
dep_simple
branch
2 times, most recently
from
4c7330c
to
9e8cc4a
Compare
Remove a few practices no longer needed do not backdate created time, let it auto-set Remove duplicate checks for update-on-launch Cache projects as well as inventory sources, combine logic Combine logic for creating project update Update dependency manager docs slightly Get rid of process_tasks we did not need
SUMMARY
I want to pull in some of the core changes from #13061 which isn't viable due to other reasons.
EDITed for changes that were done since opening
Some principles here...
get_dep_for_jobadd_dependenciesis generic to all dependency spawning, so it is moved further up in the stack which eliminates redundancy.should_update_againanswers whether a new update is needed due to latest update and cache timeout rules. It applies to update-on-launch regardless of the type (project vs inventory update), so the method is consolidated to reflect this. The old code strongly appeared to hint at different rules for different job types, but this was never the case.dependencies_processed=Truein the main loopISSUE TYPE
COMPONENT NAME