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

Updated apache_module for backward compatible. #33613

Conversation

abednarik
Copy link
Contributor

What does this PR do?

What issues does this PR fix or reference?

This introduced a change of functions names that break this module when using Beryllium or older and upgrading to Boron.
Added here a deprecated warning in to order to advice users to migrate to new functions names. Also
documentation updated since was not updated in the original commit.

Previous Behavior

State:

Enable cgi module:
  apache_module.enable:
    - name: cgi

Result:

salt '*' state.highstate
salt:
----------
          ID: Enable cgi module
    Function: apache_module.enable
        Name: cgi
      Result: False
     Comment: State 'apache_module.enable' was not found in SLS 'default'
              Reason: 'apache_module.enable' is not available.
     Started:
    Duration:
     Changes:

Summary for salt
------------
Succeeded: 0
Failed:    1
------------
Total states run:     1

New Behavior

State:

Enable cgi module:
  apache_module.enable:
    - name: cgi

Result

salt:
----------
          ID: Enable cgi module
    Function: apache_module.enable
        Name: cgi
      Result: True
     Comment:
     Started: 20:27:45.885109
    Duration: 91.94 ms
     Changes:
              ----------
              new:
                  cgi
              old:
                  None

Summary for salt
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1

Tests written?

Tests are already in place for new named: enabled / disabled. Functions added are just wrappers.

[This](saltstack@f3a2b27) introduced
a change of functions names that break this module when using Beryllium or older and upgrading to Boron.
Added here a deprecated warning in to order to advice users to migrate to new functions names. Also
documentation updated since was not updated in the original commit.
@cachedout
Copy link
Contributor

@abednarik Thank you so much for taking the time to fix this. I can't believe I let that change sneak in. I'm glad you're on the case!

@cachedout cachedout merged commit d43a9fd into saltstack:develop May 31, 2016
@rallytime
Copy link
Contributor

I think that this warning should go until at least Nitrogen to give people a change to switch their states over. I can grab that quickly.

@rallytime rallytime added the ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch. label May 31, 2016
@abednarik
Copy link
Contributor Author

abednarik commented May 31, 2016

Thanks both!

Edited: not needed to do another PR.

@abednarik abednarik deleted the abednarik_apache_module_backward_compatible branch May 31, 2016 15:53
@rallytime
Copy link
Contributor

@abednarik That's ok! I fixed it in the back-port I did of this change to 2016.3. The Nitrogen change will come through in the next merge-foward. Thanks again!

rallytime pushed a commit that referenced this pull request May 31, 2016
* Updated apache_module for backward compatible.

[This](f3a2b27) introduced
a change of functions names that break this module when using Beryllium or older and upgrading to Boron.
Added here a deprecated warning in to order to advice users to migrate to new functions names. Also
documentation updated since was not updated in the original commit.

* Move warnutil from Carbon to Nitrogen

Let's give people a little more time to update their states.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ZZZ[Done]-back-ported-bf RETIRED The pull request has been back-ported to an older branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants