Auto publish and distribute #1084
Conversation
|
WARNING!!! This PR is not attached to an issue. In most cases this is not advisable. Please see our PR docs for more information about how to attach this PR to an issue. |
dralley
force-pushed
the
auto-publish-distribute
branch
4 times, most recently
from
f6487fc
to
679b00d
Compare
dralley
force-pushed
the
auto-publish-distribute
branch
8 times, most recently
from
f60b857
to
3cbb79b
Compare
Required PR: pulp/pulpcore#1084 closes: #7469 https://pulp.plan.io/issues/7469
Required PR: pulp/pulpcore#1084 closes: #7622 https://pulp.plan.io/issues/7622
Required PR: pulp/pulpcore#1084 closes: #7469 https://pulp.plan.io/issues/7469
dralley
force-pushed
the
auto-publish-distribute
branch
2 times, most recently
from
98ac024
to
3a9f98c
Compare
Required PR: pulp/pulpcore#1084 closes: #7469 https://pulp.plan.io/issues/7469
Required PR: pulp/pulpcore#1084 closes: #7622 https://pulp.plan.io/issues/7622
Required PR: pulp/pulpcore#1084 closes: #7469 https://pulp.plan.io/issues/7469
Required PR: pulp/pulpcore#1084 closes: #7622 https://pulp.plan.io/issues/7622
dralley
force-pushed
the
auto-publish-distribute
branch
from
3a9f98c
to
7fad89f
Compare
Required PR: pulp/pulpcore#1084 closes: #7469 https://pulp.plan.io/issues/7469
Required PR: pulp/pulpcore#1084 closes: #7622 https://pulp.plan.io/issues/7622
Required PR: pulp/pulpcore#1084 [noissue]
Required PR: pulp/pulpcore#1084 [noissue]
Required PR: pulp/pulpcore#1084 [noissue]
Required PR: pulp/pulpcore#1084 [noissue]
Required PR: pulp/pulpcore#1084 [noissue]
Required PR: pulp/pulpcore#1084 [noissue]
Required PR: pulp/pulpcore#1084 [noissue]
Required PR: pulp/pulpcore#1084 [noissue]
Required PR: pulp/pulpcore#1084 [noissue]
Required PR: pulp/pulpcore#1084 [noissue]
Required PR: pulp/pulpcore#1084 [noissue]
Required PR: pulp/pulpcore#1084 closes: #7469 https://pulp.plan.io/issues/7469
Required PR: pulp/pulpcore#1084 closes: #7622 https://pulp.plan.io/issues/7622
Required PR: pulp/pulpcore#1084 closes: #7469 https://pulp.plan.io/issues/7469
Required PR: pulp/pulpcore#1084 [noissue]
dralley
force-pushed
the
auto-publish-distribute
branch
from
3adea79
to
ceec3a3
Compare
| def create(self, validated_data): | ||
| """Override the default create to use get_or_create behavior.""" | ||
| instance, _ = self.Meta.model.objects.get_or_create(**validated_data) | ||
| return instance |
There was a problem hiding this comment.
I'm open to disagreement on the way this works, maybe it's a step too far.
The idea here is that we now effectively have all the publish settings duplicated between *Publication and *PublishSettings objects, which is unfortunate. One way we could unify everything is by moving the data into PublishSettings exclusively, and attaching the PublishSettings to the Publication.
But that would also lead to a lot of duplicate PublishSettings objects unless we implement get_or_create() semantics, which is why I did this.
But if we just ignore the duplication of fields, it does avoid all of these changes, at the expense of the data model being a little less consistent.
There was a problem hiding this comment.
Can you clarify for me: with the code I see in this PR this does have everything live on PublishSettings exclusively yes?
There was a problem hiding this comment.
Every plugin went and added fields to their own Publication models like this: https://github.com/pulp/pulp_rpm/blob/master/pulp_rpm/app/models/repository.py#L310-L325
This is how the user can know what settings were used in the creation of their publication.
Now that we have a PublishSettings model, all of those fields are duplicated in the plugins.
The question is, do we leave it like that, or do we try to move towards just creating a PublishSettings any time you publish (with get_or_create() semantics to avoid spamming them) and including that on the publication.
There was a problem hiding this comment.
Maybe I misread your question. Most of the code related to this lives here, but under this framework one thing that would change is that the publish viewset would always get_or_create() a publish settings object and then pass it into the publish task instead of individual settings.
dralley
force-pushed
the
auto-publish-distribute
branch
from
ceec3a3
to
78d7a96
Compare
CHANGES/plugin_api/7626.feature
Outdated
| @@ -0,0 +1 @@ | |||
| Added a new model `PublishSettings` which plugin writers are expected to subclass. When dispatching a task to create a `Publication`, use ".get_or_create()" to create a `PublishSettings` object and refer your publish tasks to use it. When creating a `Publication` inside your publish task with `Publication.create()`, you should pass the in the `PublishSettings` object so that it may be associated with the `Publication`. A new method `on_new_version()` should be added to repository models to define behavior that takes place when new repository versions are created. For an example of these changes, please see: https://github.com/pulp/pulp_file/pull/467 | |||
There was a problem hiding this comment.
Can this line-wrap at 100 chars please?
There was a problem hiding this comment.
Also sphinx needs double backticks instead of single for rst formatting
There was a problem hiding this comment.
This content is great. We also need this to go into a new section of docs in the plugin writer guide I think. Please add that too.
dralley
force-pushed
the
auto-publish-distribute
branch
from
78d7a96
to
dab8514
Compare
dralley
force-pushed
the
auto-publish-distribute
branch
2 times, most recently
from
fb439fa
to
805551e
Compare
Required PR: pulp/pulpcore#1084 closes: #7469 https://pulp.plan.io/issues/7469
Required PR: pulp/pulp_file#467 closes: #7626 https://pulp.plan.io/issues/7626
dralley
force-pushed
the
auto-publish-distribute
branch
from
805551e
to
65fdab8
Compare
Required PR: pulp/pulpcore#1084 closes: #7469 https://pulp.plan.io/issues/7469
Required PR: pulp/pulpcore#1084 [noissue]
This is like 10% finished, I just want to discuss ideas