Skip to content

Enable dispatcherd under feature flag 1st iteration - AAP-46009 #1308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

Alex-Izquierdo
Copy link
Collaborator

@Alex-Izquierdo Alex-Izquierdo commented May 16, 2025

Initial minimal implementation of dispatcherd under the feature flag. Current rq deployment tested
Still missing tests for dispatcherd and proxies that we are going to address in later iterations as part of https://issues.redhat.com/browse/AAP-46008

Dispatcherd is widely tested as well but not guaranteed a this point.
WE NEED TO CONFIRM THE AVAILABILITY OF THE RPM PRIOR TO MERGE THIS, (tracked internally in https://issues.redhat.com/browse/AAP-45706)

Jira: https://issues.redhat.com/browse/AAP-46009

@Alex-Izquierdo Alex-Izquierdo force-pushed the enable-dispatcherd-1st-iteration branch from 3ddc037 to 83c78ab Compare May 19, 2025 15:19
@codecov-commenter
Copy link

codecov-commenter commented May 19, 2025

Codecov Report

Attention: Patch coverage is 83.61345% with 39 lines in your changes missing coverage. Please review.

Project coverage is 94.38%. Comparing base (77a0b8e) to head (be870c4).

Files with missing lines Patch % Lines
src/aap_eda/core/tasking/__init__.py 70.21% 14 Missing ⚠️
src/aap_eda/tasks/project.py 75.75% 8 Missing ⚠️
src/aap_eda/tasks/orchestrator.py 60.00% 6 Missing ⚠️
src/aap_eda/core/management/commands/rqworker.py 68.75% 5 Missing ⚠️
src/aap_eda/tasks/analytics.py 75.00% 2 Missing ⚠️
tests/integration/test_advisory_lock.py 93.93% 2 Missing ⚠️
src/aap_eda/api/views/mixins.py 66.66% 1 Missing ⚠️
src/aap_eda/services/activation/engine/podman.py 87.50% 1 Missing ⚠️
@@            Coverage Diff             @@
##             main    #1308      +/-   ##
==========================================
- Coverage   94.56%   94.38%   -0.18%     
==========================================
  Files         318      318              
  Lines       18606    18742     +136     
==========================================
+ Hits        17594    17690      +96     
- Misses       1012     1052      +40     
Flag Coverage Δ
unit-int-tests-3.11 94.32% <83.19%> (-0.18%) ⬇️
unit-int-tests-3.12 94.38% <83.61%> (-0.18%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/aap_eda/api/views/eda_credential.py 99.10% <100.00%> (ø)
src/aap_eda/api/views/project.py 96.87% <100.00%> (ø)
src/aap_eda/core/apps.py 86.66% <100.00%> (+3.33%) ⬆️
.../aap_eda/services/activation/activation_manager.py 63.40% <100.00%> (+0.17%) ⬆️
src/aap_eda/services/activation/restart_helper.py 90.47% <ø> (ø)
src/aap_eda/services/project/imports.py 87.89% <100.00%> (+0.07%) ⬆️
src/aap_eda/settings/core.py 100.00% <100.00%> (ø)
src/aap_eda/settings/post_load.py 95.95% <100.00%> (+0.07%) ⬆️
src/aap_eda/wsapi/consumers.py 92.94% <ø> (-0.09%) ⬇️
tests/integration/api/test_eda_credential.py 100.00% <ø> (ø)
... and 15 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Alex-Izquierdo Alex-Izquierdo marked this pull request as ready for review May 20, 2025 14:37
@Alex-Izquierdo Alex-Izquierdo requested a review from a team as a code owner May 20, 2025 14:37
@Alex-Izquierdo Alex-Izquierdo changed the title WIP - Enable dispatcherd under feature flag 1st iteration Enable dispatcherd under feature flag 1st iteration May 20, 2025
@Alex-Izquierdo Alex-Izquierdo changed the title Enable dispatcherd under feature flag 1st iteration Enable dispatcherd under feature flag 1st iteration - AAP-46009 May 20, 2025
@Alex-Izquierdo Alex-Izquierdo requested a review from simaishi May 20, 2025 14:44
Alex-Izquierdo added a commit that referenced this pull request May 21, 2025
@Alex-Izquierdo Alex-Izquierdo force-pushed the enable-dispatcherd-1st-iteration branch from dd15056 to de748bf Compare May 21, 2025 17:04
Alex-Izquierdo added a commit that referenced this pull request May 21, 2025
Alex-Izquierdo and others added 13 commits May 22, 2025 20:32
<!-- Mandatory: Provide a clear, concise description of the changes and
their purpose -->
<!-- If applicable, provide a link to the issue that is being addressed
-->

<!-- What is being changed? -->
<!-- Why is this change needed? -->
<!-- How does this change address the issue? -->
<!-- Does this change introduce any new dependencies, blockers or
breaking changes? -->
<!-- How it can be tested? -->

Signed-off-by: Alex <[email protected]>
<!-- Mandatory: Provide a clear, concise description of the changes and
their purpose -->
<!-- If applicable, provide a link to the issue that is being addressed
-->

<!-- What is being changed? -->
<!-- Why is this change needed? -->
<!-- How does this change address the issue? -->
<!-- Does this change introduce any new dependencies, blockers or
breaking changes? -->
<!-- How it can be tested? -->

Signed-off-by: Alex <[email protected]>
<!-- Mandatory: Provide a clear, concise description of the changes and
their purpose -->
<!-- If applicable, provide a link to the issue that is being addressed
-->

<!-- What is being changed? -->
<!-- Why is this change needed? -->
<!-- How does this change address the issue? -->
<!-- Does this change introduce any new dependencies, blockers or
breaking changes? -->
<!-- How it can be tested? -->

---------

Signed-off-by: Alex <[email protected]>
<!-- Mandatory: Provide a clear, concise description of the changes and
their purpose -->
<!-- If applicable, provide a link to the issue that is being addressed
-->

<!-- What is being changed? -->
<!-- Why is this change needed? -->
<!-- How does this change address the issue? -->
<!-- Does this change introduce any new dependencies, blockers or
breaking changes? -->
<!-- How it can be tested? -->

---------

Signed-off-by: Alex <[email protected]>
<!-- Mandatory: Provide a clear, concise description of the changes and
their purpose -->
<!-- If applicable, provide a link to the issue that is being addressed
-->

<!-- What is being changed? -->
<!-- Why is this change needed? -->
<!-- How does this change address the issue? -->
<!-- Does this change introduce any new dependencies, blockers or
breaking changes? -->
<!-- How it can be tested? -->

---------

Signed-off-by: Alex <[email protected]>
<!-- Mandatory: Provide a clear, concise description of the changes and
their purpose -->
<!-- If applicable, provide a link to the issue that is being addressed
-->

<!-- What is being changed? -->
<!-- Why is this change needed? -->
<!-- How does this change address the issue? -->
<!-- Does this change introduce any new dependencies, blockers or
breaking changes? -->
<!-- How it can be tested? -->

---------

Signed-off-by: Alex <[email protected]>
Signed-off-by: Alex <[email protected]>
bzwei and others added 16 commits May 22, 2025 20:32
Update job uniqueness tests to mock proxy functions. 
Also moves import/sync project tasks from the job decorator to the
direct calling, this for consistency and convenience at testing.
(projects jobs continue working as expected, manually verified)

Signed-off-by: Alex <[email protected]>
Signed-off-by: Alex <[email protected]>
<!-- Mandatory: Provide a clear, concise description of the changes and
their purpose -->
<!-- If applicable, provide a link to the issue that is being addressed
-->

<!-- What is being changed? -->
<!-- Why is this change needed? -->
<!-- How does this change address the issue? -->
<!-- Does this change introduce any new dependencies, blockers or
breaking changes? -->
<!-- How it can be tested? -->

Signed-off-by: Alex <[email protected]>
<!-- Mandatory: Provide a clear, concise description of the changes and
their purpose -->
<!-- If applicable, provide a link to the issue that is being addressed
-->

<!-- What is being changed? -->
<!-- Why is this change needed? -->
<!-- How does this change address the issue? -->
<!-- Does this change introduce any new dependencies, blockers or
breaking changes? -->
<!-- How it can be tested? -->

Signed-off-by: Alex <[email protected]>
As per discussion with @AlanCoding, rename hazmat module and remove
"metrics" extra, which is not currently used and eventually won't be
needed.

Signed-off-by: Alex <[email protected]>
Signed-off-by: Alex <[email protected]>
Follow up #1312, add missing
ref in conf

Signed-off-by: Alex <[email protected]>
<!-- Mandatory: Provide a clear, concise description of the changes and
their purpose -->
<!-- If applicable, provide a link to the issue that is being addressed
-->

<!-- What is being changed? -->
<!-- Why is this change needed? -->
<!-- How does this change address the issue? -->
<!-- Does this change introduce any new dependencies, blockers or
breaking changes? -->
<!-- How it can be tested? -->

Signed-off-by: Alex <[email protected]>
Signed-off-by: Alex <[email protected]>
Signed-off-by: Alex <[email protected]>
@Alex-Izquierdo Alex-Izquierdo force-pushed the enable-dispatcherd-1st-iteration branch from 528f6d7 to be870c4 Compare May 22, 2025 18:32
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
5.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@@ -35,12 +41,29 @@ def add_arguments(self, parser: CommandParser) -> None:

def handle(self, *args, **options) -> None:
if features.DISPATCHERD:
self.stderr.write(
if "worker_class" not in options:
Copy link
Contributor

@mkanoor mkanoor May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alex-Izquierdo Can this code block be moved into something like handle_dispatcherd

# DISPATCHERD SETTINGS
# ---------------------------------------------------------

DISPATCHERD_DEFAULT_CHANNEL = "default"
Copy link
Contributor

@mkanoor mkanoor May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alex-Izquierdo Will this channel name have conflicts with other services like controller should we have a eda prefix for this?

@tasking.redis_connect_retry()
def sync_project_rq(project_id: int):
queue = django_rq.get_queue(name=PROJECT_TASKS_QUEUE)
return queue.enqueue(_sync_project, project_id=project_id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alex-Izquierdo can this return id to be consistent with sync_project_dispatcherd

     return queue.enqueue(_sync_project, project_id=project_id).id

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants