This repository has been archived by the owner on Dec 7, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 36
use pulp-manager to reset db #91
Merged
Merged
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
- name: makemigrations | ||
command: "{{pulp_venv}}/bin/pulp-manager makemigrations {{app_label}}" | ||
become: false |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
- name: migrate | ||
command: "{{pulp_venv}}/bin/pulp-manager {{item}}" | ||
with_items: | ||
- "reset_db --noinput" | ||
- "migrate auth --noinput" | ||
- "migrate --noinput" | ||
become: false |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
- name: Install Plugin packages into core Pulp Python environment | ||
pip: | ||
name: "{{ pulp_devel_dir }}/{{ plugin_name }}" | ||
extra_args: "-e" | ||
virtualenv: "{{ pulp_venv }}" | ||
become: false | ||
|
||
# variables plugin_name and app_label are passed implicitly | ||
- include: roles/django_db/tasks/makemigrations.yml | ||
- include: roles/django_db/tasks/reset.yml |
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey so I found an issue with this. If you provide the names of the plugins up-front instead of letting them be enumerated, this fails if the plugin isn't in the user's devel directory.
Specifically, I forgot to clone the pulp_file plugin, and the ansible script failed out with
I assume we want to keep the previous workflow where you don't necessarily need to have the plugins cloned - but let me know if that assumption is invalid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have 2 choices. We can use the task to assert that the repo exists:
https://github.com/pulp/devel/blob/3.0-dev/ansible/roles/common/tasks/require_repo.yml
Or we can leave this role commented out as the default. I'm inclined to go that route. Pretty sure you could clone, uncomment and run
vagrant provision
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of letting the presence of pulp_file be optional, which would mean having this line of code commented out by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair, although it means that devs would need to uncomment/list all plugins they want to be installed themselves, which is probably fine at this point in the Pulp 3 lifecycle. I feel like we would eventually want to go back to the old model installing everything it could find, but maybe now isn't the time to worry about it.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True enough. I do like the idea of making plugin installation as seamless and easy as possible. We both want the same end goal.
I want to get to a place where we can install pulpcore (with development extras) with a single button press as soon as possible. This will be much easier to do if we start by putting a pulpcore-only playbook in front of the user by default.
EDIT: One specific possible solution is to create a role (or roles) for installing plugins, and to make that role(s) call this role after the plugin(s) is installed. That would place the logic for installing the plugin near the logic for running that plugin's migrations. Regardless of how exactly it happens, the end procedure is the same: get one-button pulpcore installations working now, and get one-button plugin installations working later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long story short, we were already hardcoding pulp_file into the ansible install, so for now, I think its best to move that out of the role and into the playbook.
Id like to comment out the pulp_file install in the default playbook and use require_repo to fail with an easily readable message. (Probably also a small docs change). That seems like the safest default to me, and we can continue this conversation on the pulp-dev list to align our longer term ideas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me. Let me know when those changes have been made and I will take another look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ichimonji10, yes, and that part also depends on a hardcoded list in pulp_facts. I prefer that if someone wants to modify their dev environment to install their new plugin, they should only need to change the playbook.