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

Added worker command. #144

Merged
merged 1 commit into from
Feb 15, 2021
Merged

Added worker command. #144

merged 1 commit into from
Feb 15, 2021

Conversation

daviddavis
Copy link
Contributor

fixes #144

daviddavis pushed a commit to daviddavis/pulp-cli that referenced this pull request Feb 13, 2021
daviddavis pushed a commit to daviddavis/pulp-cli that referenced this pull request Feb 13, 2021
daviddavis pushed a commit to daviddavis/pulp-cli that referenced this pull request Feb 13, 2021
@@ -0,0 +1 @@
Added worker command.
Copy link
Member

Choose a reason for hiding this comment

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

Meh, self referencing does not seem to work. But i like the idea of having a changelog without the need to file an issue just to say: This is what i do in the PR anyway.

OTOH, i thought gh handles PRs as a special sort of issues.

It should work however if you do not put the fixes in the commit message. Which means we should add more checking for spurious changelog snippets. What if "(no [noissue]) and (no fixes #??)" implies we need one for the PR number?

log.out Outdated
@@ -0,0 +1 @@
[{"pulp_created": "2021-02-13T13:33:04.220502Z", "pulp_href": "/pulp/api/v3/workers/3754d4ea-b554-432d-b20e-45026ce46c12/", "name": "resource-manager", "last_heartbeat": "2021-02-13T22:16:02.245766Z"}]
Copy link
Member

Choose a reason for hiding this comment

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

Those files i would not expect here. Is the cwd mechanism not working for you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know. I re-ran the tests and it didn't appear. Will keep an eye out.

]

worker.add_command(list_command(decorators=filter_options))
worker.add_command(show_command(decorators=[href_option, name_option]))
Copy link
Member

Choose a reason for hiding this comment

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

Whenever i think, this is a lot of boilerplate, i need to remind myself, that it would be ca. 8-16 times the code if spelled out here in more detail...

click.option("--name-contains", "name__contains"),
click.option("--name-icontains", "name__icontains"),
click.option("--missing/--not-missing", default=None),
click.option("--online/--not-online", default=None),
Copy link
Member

Choose a reason for hiding this comment

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

So this is how you make a tristate option. 👍

daviddavis pushed a commit to daviddavis/pulp-cli that referenced this pull request Feb 15, 2021
Copy link
Member

@mdellweg mdellweg left a comment

Choose a reason for hiding this comment

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

This all looks very good. We just need to find a convenient way to add changelogs without issues.

daviddavis pushed a commit to daviddavis/pulp-cli that referenced this pull request Feb 15, 2021
@daviddavis daviddavis force-pushed the worker branch 2 times, most recently from 0e44b17 to fe2a4ba Compare February 15, 2021 16:12
@mdellweg
Copy link
Member

The approval stands. ;)

@daviddavis daviddavis merged commit 843499d into pulp:develop Feb 15, 2021
@mdellweg mdellweg added this to the 0.5.0 milestone Feb 17, 2021
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