Skip to content

Conversation

@a76yyyy
Copy link

@a76yyyy a76yyyy commented Mar 17, 2025

What does this PR do?

Fixes incorrect storage of _return-type job results in local_cache when using multi_returner configuration. Now strictly follows the logic in salt.utils.job.store_job by only executing multi-returner storage when load['cmd'] is not equal to '_return'.

What issues does this PR fix or reference?

None

Previous Behavior

With multi-returner configuration:

master_job_cache: multi_returner
multi_returner:
  - local_cache
  - elasticsearch # or other

All types of return data (including cmd: _return job results) were stored in local_cache, violating the expected behavior defined in store_job logic:

salt/salt/utils/job.py

Lines 127 to 130 in f906ca5

save_load = True
if job_cache == "local_cache" and mminion.returners[getfstr](load.get("jid", "")):
# The job was saved previously.
save_load = False

New Behavior

Now only save_load when returner_ == "local_cache" and load['cmd'] != '_return'

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

No

@a76yyyy a76yyyy requested a review from a team as a code owner March 17, 2025 10:34
@welcome
Copy link

welcome bot commented Mar 17, 2025

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject.pdl@broadcom.com. We’re glad you’ve joined our community and look forward to doing awesome things with you!

Copy link
Contributor

@twangboy twangboy left a comment

Choose a reason for hiding this comment

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

Please add a changelog and some tests

Copy link
Contributor

@twangboy twangboy left a comment

Choose a reason for hiding this comment

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

Please write a test for this

@twangboy twangboy added needs-testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases test:full Run the full test suite labels Jun 27, 2025
@twangboy twangboy added this to the Argon v3008.0 milestone Jun 27, 2025
@dwoz
Copy link
Contributor

dwoz commented Aug 22, 2025

Please write a test for this

@a76yyyy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-testcase PR needs test cases written, or the issue is about a bug/feature that needs test cases test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants