-
Notifications
You must be signed in to change notification settings - Fork 107
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 support for automatic publishing #1229
Conversation
65fdab8
to
c90cf13
Compare
You'll notice that this implementation is a lot different (simpler) than it was before, and that I ditched PublishSettings objects entirely. Basically, as I thought about it more and more, I talked myself out of the whole idea, despite being the one of the ones pushing for it initially. I went with There are many reasons why but the simplest is:
So adding this whole new thing in core, and trying to manage even more relationships, and making 75% of plugins more tricky to use for an architecture that would only benefit some very niche use cases (sharing publish settings) for maybe 1/3 of the plugins, just doesn't make a whole lot of sense to me anymore. The cure is worse than the disease I think. There's other reasons too which I can explain on a call or something. |
Required PR: pulp/pulpcore#1229 closes: #7469 https://pulp.plan.io/issues/7469
c90cf13
to
42b6bf0
Compare
Required PR: pulp/pulpcore#1229 closes: #7469 https://pulp.plan.io/issues/7469
Attached issue: https://pulp.plan.io/issues/7626 |
Required PR: pulp/pulpcore#1229 closes: #7469 https://pulp.plan.io/issues/7469
0da2d86
to
5f2779d
Compare
Required PR: pulp/pulpcore#1229 [noissue]
Required PR: pulp/pulpcore#1229 closes: #7622 https://pulp.plan.io/issues/7622
Required PR: pulp/pulpcore#1229 closes: #7622 https://pulp.plan.io/issues/7622
Required PR: pulp/pulpcore#1229 closes: #7622 https://pulp.plan.io/issues/7622
Required PR: pulp/pulpcore#1229 closes: #7622 https://pulp.plan.io/issues/7622
Required PR: pulp/pulpcore#1229 closes: #7622 https://pulp.plan.io/issues/7622
Required PR: pulp/pulpcore#1229 closes: #7622 https://pulp.plan.io/issues/7622
@@ -25,7 +25,7 @@ class Migration(migrations.Migration): | |||
('content_guard', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='core.ContentGuard')), | |||
('publication', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='core.Publication')), | |||
('remote', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='core.Remote')), | |||
('repository', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='core.Repository')), | |||
('repository', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='core.Repository', related_name='distributions')), |
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.
Not entirely sure, if this should be a new migration.
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 believe this is an automatic one produced with 'makemigrations'
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.
It is modifying a migration that is on master, but not yet released. I'm not saying we should not do this, however, just asking.
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.
hmm i see what you're saying - i think in the past practice we'd just have a migration on top instead of modifying an unreleased one.
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 can do that if there's a strong desire to. I just thought that since this work and Brian's is so closely linked, there's not a great need to create an entire new migration just for this one-line change that I probably should have asked for on his PR initially.
So, not a strong opinion, just convenience.
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.
Not a strong opinion here.
5f2779d
to
b689e0e
Compare
CHANGES/plugin_api/7626.feature
Outdated
@@ -0,0 +1 @@ | |||
Added a new callback method to `Repository` named `on_new_version()`, which runs when a new repository version has been created. This can be used for e.g. automatically publishing or distributing a new repository version after it has been created. |
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.
@daviddavis How is this
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.
Added a new callback method to `Repository` named `on_new_version()`, which runs when a new repository version has been created. This can be used for e.g. automatically publishing or distributing a new repository version after it has been created. | |
Added a new callback method to ``Repository`` named ``on_new_version()``, which runs when a new repository version has been created. This can be used for e.g. automatically publishing or distributing a new repository version after it has been created. |
b689e0e
to
23bb55c
Compare
Required PR: pulp/pulp_file#492 closes: #7626 https://pulp.plan.io/issues/7626
23bb55c
to
571655d
Compare
Required PR: pulp/pulpcore#1229 [noissue]
closes: #7626
https://pulp.plan.io/issues/7626