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

Add export job function #30

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jon-rtr
Copy link
Contributor

@jon-rtr jon-rtr commented Jan 23, 2020

PyCharm shows error messages like Shadows name 'username' from outer scope in the init function of the Rundeck class. We can disambiguate the parameters to the init function from the
variables in the main context by renaming the variables. In this
instance, I appended an underscore to the variables.

Add a function that exposes Rundeck API functionality for exporting an
individual job.

Ignore JetBrains IDE-generated files.
Run the black formatter over the code.
As of 2020 Jan 01, official support for Python 2 has ended (https://www.python.org/doc/sunset-python-2/, https://python3statement.org/).

* Remove `from __future__ import absolute_import` statement.
* Remove `from __future__ import unicode_literals` statement.
* Remove `from __future__ import print_function` statement.
* Remove `from urlparse import urljoin` statement, and retain the Python
3 equivalent.
* Switch to using format strings.
* BUGFIX: Per https://docs.rundeck.com/docs/api/rundeck-api.html#bulk-delete-executions, the url in the bulk_delete_executions function should look like `{}/executions/delete`, not `{}/executions/{}/delete`.
From rundeck/rundeck#1923:
> I've traced the issue down to the Content-Type header that messes up the whole request (It's not needed for a GET anyway, was there from a previous POST). The output is still a XML, I'll open a different issue for that

There are a few posts saying the same thing: GET requests specifying a
Content-Type header causes the URL parameters to be ignored.
There are some cases where end-users of the library want to override the
headers sent to Rundeck (ex. not every response is going to be
application/json).
Per https://docs.rundeck.com/docs/api/rundeck-api.html#execution-query,
we can pass a format parameter (json, xml, text) and specify the
relevant Accept header so that you can, for example, download the raw
log text.
PyCharm shows error messages like `Shadows name 'username' from outer scope` in the init function of the Rundeck class.

We can disambiguate the parameters to the init function from the
variables in the main context by renaming the variables. In this
instance, I appended an underscore to the variables.
Add a function that exposes Rundeck API functionality for exporting an
individual job.
@jon-rtr jon-rtr requested a review from pschmitt January 23, 2020 03:50
Copy link
Owner

@pschmitt pschmitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's quite hard to review 4 PRs that basically all include each other.

self.rundeck_url = rundeck_url
self.API_URL = urljoin(rundeck_url, '/api/{}'.format(api_version))
self.API_URL = urljoin(rundeck_url, f"/api/{api_version}")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

f strings are Python 3.6+ only. I happen to use these quite a lot as well but I'm not sure if requiring 3.6+ is that smart of a move.

@@ -1,2 +1 @@
from __future__ import absolute_import
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR seems to include #27
Is this intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @pschmitt I needed to make a ton of changes to make this library useful to me so I broke them up into separate branches instead of one massive go, and then pushed them up. I'll respond to this better when I have a moment later today. Cheers.

url = urljoin(self.rundeck_url, '/j_security_check')
p = {'j_username': self.username, 'j_password': self.password}
url = urljoin(self.rundeck_url, "/j_security_check")
p = {"j_username": self.username, "j_password": self.password}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And #26 as well?

@pschmitt pschmitt added the needs-rebase PR needs to be rebase against the current master label Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-rebase PR needs to be rebase against the current master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants