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

Facilitate pulp_workers config #303

Merged
merged 1 commit into from May 27, 2020
Merged

Facilitate pulp_workers config #303

merged 1 commit into from May 27, 2020

Conversation

fao89
Copy link
Member

@fao89 fao89 commented May 26, 2020

@fao89 fao89 requested a review from dralley May 26, 2020 12:21
@pulpbot
Copy link
Member

pulpbot commented May 26, 2020

Attached issue: https://pulp.plan.io/issues/6774

@@ -2,3 +2,4 @@
pulp_requirements_dir: "{{ pulp_source_dir }}"
pulp_devel_package_retries: 5
pulp_devel_supplement_bashrc: false
pulp_workers: 2
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: can this have a newline at the end of it. If someone were to concat this with another yml file it would break without it.

state: started
enabled: true
pulp_workers: 2
default_worker:
Copy link
Member

Choose a reason for hiding this comment

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

I imagined we would not be a user option. It would be useful if a user wanted to create a bunch of systemd units but then start them another way. To me that is rare enough and would require extra work from the user that I don't think adding is valuable. That's just my take; others could feel differently.

Copy link
Member

@bmbouter bmbouter left a comment

Choose a reason for hiding this comment

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

This looks good to me. @mikedep333 and @dkliban ?

@bmbouter
Copy link
Member

Thank you @fao89 !

@fao89 fao89 requested a review from mikedep333 May 26, 2020 21:31
Copy link
Member

@mikedep333 mikedep333 left a comment

Choose a reason for hiding this comment

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

Awesome! Just see comment about the change message.

name: pulpcore-worker@{{ item }}.service
state: started
enabled: true
with_sequence: 'start=1 count={{ pulp_workers }}'
Copy link
Member

Choose a reason for hiding this comment

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

Nice syntax!

roles/pulp_workers/README.md Show resolved Hide resolved
@@ -1,7 +1,7 @@
# If adding new functions to this file, note that you can add help text to the function
# by defining a variable with name _<function>_help containing the help text

SERVICES=("pulpcore-content pulpcore-worker@1 pulpcore-worker@2 pulpcore-resource-manager pulpcore-api")
SERVICES=("pulpcore-content {% for num in range(pulp_workers) %}pulpcore-worker@{{ num + 1 }} {% endfor %}pulpcore-resource-manager pulpcore-api")
Copy link
Member

Choose a reason for hiding this comment

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

Nice syntax!

@@ -0,0 +1 @@
Facilitate `pulp_workers` config
Copy link
Member

Choose a reason for hiding this comment

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

I think we should have a more specific message.

Copy link
Member

Choose a reason for hiding this comment

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

@mikedep333 can you suggest something

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Facilitate `pulp_workers` config
Replaced `pulp_workers` dictionary variable with the `pulp_workers` integer variable. `pulp_workers` is now simply the number of workers.

@@ -0,0 +1 @@
Facilitate `pulp_workers` config
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Facilitate `pulp_workers` config
Replaced `pulp_workers` dictionary variable with the `pulp_workers` integer variable. `pulp_workers` is now simply the number of workers.

@mikedep333 mikedep333 merged commit 140d400 into pulp:master May 27, 2020
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.

None yet

4 participants