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

[PR #51363] Added an implementation of event_return for the etcd returner (returners/etcd_return.py) #51346

Closed
wants to merge 18 commits into from

Conversation

arizvisa
Copy link
Contributor

What does this PR do?

This implements event_return for the etcd returner which allows etcd to be specified via the event_return option.

What issues does this PR fix or reference?

This PR is dependent on issue #51345 being fixed as it seems that etcd.set doesn't return an EtcdResult (or raise exceptions anymore in favor of None), and there's a number of things that the python-etcd module uses in order to inform the caller how to handle it. This PR is marked WIP pending that issue being fixed.

New Behavior

Adds support for using etcd via the event_return option, instead of being forced to lookup jobs.

Tests written?

:-J

@dwoz
Copy link
Contributor

dwoz commented Jan 27, 2019

@arizvisa can you please check on the linter errors and see if the failed tests are related to your change here?

@arizvisa
Copy link
Contributor Author

It looks like the lint errors are related to a uuid module that I imported, a '%r' being used in a format string, and whitespace issues. Let's see how it goes this time...

Can I use formatspecs for this? Most instances that call the log functions use formatstrings which is why I chose to use it...

@arizvisa
Copy link
Contributor Author

Btw, this is dependent on the etcd returner module being fixed as per #51345. I'll submit my PR for that issue in a second here.

@arizvisa
Copy link
Contributor Author

As a reminder, this PR is dependent on #51363 being merged. When (or if) that happens, I'll remove the WIP tag from this (unless y'all want to manage that dependency).

…d into salt.utils but didn't test anything and hid all the exceptions. To restore functionality we just pass through to the client attribute.
…table manner by ensuring None is always returned (the caller seems to depend on this) and empty strings aren't deserialized.
…hing the EtcdKeyNotFound exception, added a ton of comments, and debug + trace logs.
…organized the trace logs to output the key that was returned from EtcdResult, and re-worded the logs so they use the word "job" when referring to a jid.
… exceptions when writing the job fields so it doesn't get interrupted part of the way through.
@arizvisa
Copy link
Contributor Author

arizvisa commented Feb 6, 2019

I'm closing this as I merged the into my other PR that fixes the etcd returner. It was becoming too hard to maintain both separate PRs separately since I'm using the merged version of them anyways. The other PR is also just a lot larger and so I considered it a lot less likely to get merged.

@arizvisa arizvisa closed this Feb 6, 2019
@arizvisa arizvisa deleted the etcd-event_return branch February 6, 2019 05:17
@arizvisa arizvisa changed the title [WIP] Added an implementation of event_return for the etcd returner (returners/etcd_return.py) [PR #51363] Added an implementation of event_return for the etcd returner (returners/etcd_return.py) Feb 23, 2019
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.

2 participants