Skip to content
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

[DOCS] reactors should have unquie state ids to avoid nameing conflicts. #63589

Closed
whytewolf opened this issue Jan 30, 2023 · 2 comments · Fixed by #64181
Closed

[DOCS] reactors should have unquie state ids to avoid nameing conflicts. #63589

whytewolf opened this issue Jan 30, 2023 · 2 comments · Fixed by #64181
Assignees
Labels
Documentation Relates to Salt documentation severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around time-estimate-sprint

Comments

@whytewolf
Copy link
Collaborator

Description
currently the documentation for reactors does not call out that reactors should have unique state ids. this normally might not be a problem in most cases. however it is possible to have a name collision. which will cause reactors to silently fail or not run.

the possibility of name collision is called out in the code. at

salt/salt/utils/process.py

Lines 448 to 450 in 4ec0429

# intentionally not called "apply_async" since we aren't keeping track of
# the return at all, if we want to make this API compatible with multiprocessing
# threadpool we can in the future, and we won't have to worry about name collision

Suggested Fix
document the possibility of name collision causing dropped reactors.

Location or format of documentation
https://docs.saltproject.io/en/latest/topics/reactor/index.html

Additional context
This can cause an almost undetectable issue where a reactor will just not run and no error messages being displayed.

@whytewolf whytewolf added Documentation Relates to Salt documentation needs-triage labels Jan 30, 2023
@OrangeDog
Copy link
Contributor

OrangeDog commented Jan 31, 2023

Wouldn't they be reactor ids, not state ids?

I'd also suggest that it's a bug that duplicates are not detected as they are with states.

@whytewolf
Copy link
Collaborator Author

While i would agree that name makes more sense. the chunk is still called a state. and the file is still an sls module.

Also. currently is it is expected behavior. and commented as such in the code.

@barbaricyawps barbaricyawps added severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around time-estimate-sprint and removed needs-triage labels Feb 6, 2023
@anilsil anilsil added this to the Sulfur v3006.1 milestone Apr 11, 2023
@whytewolf whytewolf self-assigned this Apr 26, 2023
@whytewolf whytewolf linked a pull request Apr 28, 2023 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Relates to Salt documentation severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around time-estimate-sprint
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants