Skip to content

manager.c: Fix erroneous reloads in UpdateConfig. #552

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

Merged

Conversation

InterLinked1
Copy link
Contributor

Currently, a reload will always occur if the
Reload header is provided for the UpdateConfig
action. However, we should not be doing a reload
if the header value has a falsy value, per the
documentation, so this makes the reload behavior
consistent with the existing documentation.

Resolves: #551

@InterLinked1
Copy link
Contributor Author

cherry-pick-to: 18
cherry-pick-to: 20
cherry-pick-to: 21

@seanbright
Copy link
Contributor

seanbright commented Jan 25, 2024

What should Reload: yes do? I feel like it should reload all modules but this change would prevent that. It’s possible I am reading the code incorrectly.

This appears to be resolved in the most recent commit.

Currently, a reload will always occur if the
Reload header is provided for the UpdateConfig
action. However, we should not be doing a reload
if the header value has a falsy value, per the
documentation, so this makes the reload behavior
consistent with the existing documentation.

Resolves: asterisk#551
@gtjoseph gtjoseph added the cherry-pick-test Trigger dry run of cherry-picks label Jan 30, 2024
@github-actions github-actions bot added cherry-pick-testing-in-progress Cherry-Pick tests in progress cherry-pick-checks-passed Cherry-Pick checks passed cherry-pick-gates-failed Cherry-Pick gates failed and removed cherry-pick-test Trigger dry run of cherry-picks cherry-pick-testing-in-progress Cherry-Pick tests in progress labels Jan 30, 2024
@asterisk-org-access-app asterisk-org-access-app bot merged commit f1a9ec4 into asterisk:master Jan 30, 2024
Copy link

Successfully merged to branch master and cherry-picked to ["18","20","21"]

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

Successfully merging this pull request may close these issues.

[bug]: manager: UpdateConfig triggers reload with "Reload: no"
4 participants