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

Check for non-existant keys when updating PagerDuty escalation policies #34705

Merged
merged 1 commit into from
Jul 19, 2016

Conversation

cmclaughlin
Copy link

@cmclaughlin cmclaughlin commented Jul 15, 2016

What does this PR do?

Allows repeat_enabled and num_loops to be passed to pagerduty_escalation_policy.present. Those fields are supported by the PagerDuty UI, but weren't supported by the pagerduty_escalation_policy state until now.

Previous Behavior

Trying to pass those fields in would cause an exception.

New Behavior

Sets those fields on new/update escalation polcies.

Tests written?

No

Following the existing code, perhaps I should have done something like
this:

objects_differ = '{0} {1} {2}'.format(k, v, '')

But I don't see the value of objects_differ used as a string, only as
as a bool... so I set it as a bool.

By the way, salt/modules/pagerduty_util.py doesn't return 'changes'..
I'll open a separate issue for that.

In particular, this allows repeat_enabled and num_loops to be passed to
pagerduty_escalation_policy.present.

Following the existing code, perhaps I should have done something like
this:

    objects_differ = '{0} {1} {2}'.format(k, v, '')

But I don't see the value of objects_differ used as a string, only as
as a bool... so I set it as a bool.

By the way, salt/modules/pagerduty_util.py doesn't return 'changes'..
I'll open a seperate issue for that.
@cmclaughlin
Copy link
Author

@bruce-lyft review please

@cachedout
Copy link
Contributor

@cmclaughlin Did you want @bruce-lyft to look at this first or are we OK to get this in?

@cmclaughlin
Copy link
Author

@cachedout All good to merge... assuming the failed tests are OK. I just copied @bruce-lyft for review since he was the original author.

@bruce-lyft
Copy link
Contributor

bruce-lyft commented Jul 19, 2016

lgtm. thanks for the ping

@rallytime rallytime merged commit 21f88fc into saltstack:develop Jul 19, 2016
gitebra pushed a commit to gitebra/salt that referenced this pull request Jul 20, 2016
* upstream/develop:
  Comment out localclient in vdadm runner
  Check for non-existant keys when updating PagerDuty escalation policies (saltstack#34705)
  Remove iteritems() references from unit/transport/*.py files
  Remove iteritems() references from unit/modules/*.py
  Remove iteritems() references from integration tests
  Add jid=req handling for mysql returner. It should also store the return jid into the jid list table.
  Updates man pages (saltstack#34756)
  Remove unnedeed config test (saltstack#34751)
  Disable test
  Update azure lib dep to match the one in cloud.clouds.msazure
  add directives example to ldap3.modify
  Add cli examples for ldap3 module
  ipset.long_range doesn't need a docstring
  Minor. Fixed CR finding.
  Regex support in publisher ACL rules.
  Loop over updated keys in non recursive update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants