feat: [AAP-46254] trigger project resync after project update #1323
Conversation
kaiokmo
force-pushed
the
AAP-46254
branch
2 times, most recently
from
183e8ed to
a54107e
Compare
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## main #1323 +/- ##
==========================================
- Coverage 93.91% 93.91% -0.01%
==========================================
Files 318 318
Lines 18741 18762 +21
==========================================
+ Hits 17601 17620 +19
- Misses 1140 1142 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
src/aap_eda/tasks/project.py
Outdated
| @@ -185,3 +185,44 @@ def _monitor_project_tasks(queue_name: str) -> None: | |||
| project.save(update_fields=["import_task_id", "modified_at"]) | |||
|
|
|||
| logger.info("Task complete: Monitor project tasks") | |||
|
|
|||
|
|
|||
| def trigger_project_sync_or_import( | |||
There was a problem hiding this comment.
IMO this logic is in the wrong place. This is not a task logic, is project logic and it should not be in this file. Deciding when to sync or import the project should be consistent with what we already do in create and sync views by calling tasks.import_project, so it should be placed in the view.
There was a problem hiding this comment.
Wouldn't that add unnecessary logic to the view, by checking whether the fields are being passed in the request and are indeed different from the fields in the existing project, and then calling the needed function?
There was a problem hiding this comment.
Also, if we leave it in the view, it means the partial_update function should check if there's an import or sync already running before triggering, right?
There was a problem hiding this comment.
Right, this is not so trivial. In /sync we already raise conflict if it is pending or running. Here we are in a similar situation. The problem is that we don't have a request queue like we have for activations, so we can not enqueue more than one task for the same project. There are two possible scenarios where this is a potential problem:
- Project is updated when it has not been imported yet at creation (unlikely but possible)
- Project is updated when a sync is already pending or running.
In both cases, there would be a possibility that the project would not be imported with the most recent data because we would not be running the import/sync task because there is another one in process that might be executed with the data prior the update.
So, at this point, I only see two possibilities:
- We raise conflict if you try to update a project that is pending or running same as we already do with sync.
- We implement a full request-queue logic, maybe better to be implemented with the future job management feature.
I would say for now we can go with option 1. Thoughts? @bzwei
There was a problem hiding this comment.
We should not raise conflict. We should wait and continue the project sync after the current has finished. Consider two users work on the same project at the same time, one issues a sync command and the other changes the project url.
There was a problem hiding this comment.
The view can compare the updated fields and decide whether to start a resync task. There is no need to detect whether git_hash exists or not. project_sync can handle the case when the previous import failed and there was no rulebook imported.
This commit causes auto sync/import of a project when one of its url or scm_branch/tag/commit is updated, without the user having to manually trigger the sync via the UI/API.
|
This commit causes auto sync/import of a project when one of its url, scm_branch, or scm_refspec is updated,
without the user having to manually trigger the sync via the UI/API.
https://issues.redhat.com/browse/AAP-46254