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

Remove unused pulpcore.plugin objects #421

Merged
merged 1 commit into from Dec 3, 2019
Merged

Conversation

bmbouter
Copy link
Member

This was done by an audit against master for pulp_rpm, pulp_container,
pulp_file, pulp_deb, pulp-certguard, and pulp_cookbook.

pulp_container will require a small update replacing its CharInFilter
usage.

https://pulp.plan.io/issues/5693
closes #5693

@bmbouter
Copy link
Member Author

pulp_container needs an update replacing its https://github.com/pulp/pulp_container/blob/c924075a7d6817840cf6a58d324680170dd09564/pulp_container/app/viewsets.py#L42 with direct usage of

class CharInFilter(BaseInFilter, CharFilter):

@goosemania
Copy link
Member

@bmbouter ,
RPM plugin uses MultipleArtifactContentSerializer and Model.
Migration plugin uses Model, IdentityField, HyperlinkRelatedFilter.

Model will become BaseModel, and what to do with the rest? Direct import?

Let's coordinate the merge/cherry-pick, so RPM plugin is not broken for long.

@@ -0,0 +1 @@
The ``pulpcore.plugin.models.Model`` is renamed to ``pulpcore.plugin.models.BaseModel``.
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 it's better to mention all the objects which are removed from the plugin API, so plugin writers are not surprised that something disappeared.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea! I will add those here.

@ipanova
Copy link
Member

ipanova commented Nov 27, 2019

@bmbouter pulp_container uses CharInFilter, since

pulp_container needs an update replacing its https://github.com/pulp/pulp_container/blob/c924075a7d6817840cf6a58d324680170dd09564/pulp_container/app/viewsets.py#L42 with direct usage of

class CharInFilter(BaseInFilter, CharFilter):

ok

@bmbouter
Copy link
Member Author

bmbouter commented Dec 2, 2019

@bmbouter ,
RPM plugin uses MultipleArtifactContentSerializer and Model.
Migration plugin uses Model, IdentityField, HyperlinkRelatedFilter.

Let me back out these changes

Model will become BaseModel, and what to do with the rest? Direct import?

Yes I'm going to try to open PRs to fix this before merging this.

Let's coordinate the merge/cherry-pick, so RPM plugin is not broken for long.

👍

@bmbouter
Copy link
Member Author

bmbouter commented Dec 2, 2019

I added back MultipleArtifactContentSerializer, IdentityField, HyperlinkRelatedFilter, and CharInField to keep things simple. So really the main change is the Model -> BaseModel. That is motivated because our API shouldn't compete with Django for the actual Model (django is the real one).

@bmbouter
Copy link
Member Author

bmbouter commented Dec 2, 2019

I also added the removals in release notes.

bmbouter added a commit to bmbouter/pulp_rpm that referenced this pull request Dec 2, 2019
pulpcore changed the name of `Model` to `BaseModel`. This PR updated
pulp_rpm to do the same.

Required PR: pulp/pulpcore#421

https://pulp.plan.io/issues/5693
closes #5693
@bmbouter
Copy link
Member Author

bmbouter commented Dec 2, 2019

Here's the corresponding pulp_rpm PR: pulp/pulp_rpm#1546 I'm making one for pulp_container now too.

bmbouter added a commit to bmbouter/pulp-2to3-migration that referenced this pull request Dec 2, 2019
pulpcore changed the name of `Model` to `BaseModel`. This PR updates
pulp-2to3-migration to do the same.

Required PR: pulp/pulpcore#421

https://pulp.plan.io/issues/5693
closes #5693
@bmbouter
Copy link
Member Author

bmbouter commented Dec 2, 2019

I realized pulp_container didn't need an update, but pulp-2to3-migraiton does: pulp/pulp-2to3-migration#53

bmbouter added a commit to bmbouter/pulp_ansible that referenced this pull request Dec 2, 2019
pulpcore changed the name of `Model` to `BaseModel`. This PR updates
pulp_ansible to do the same.

Required PR: pulp/pulpcore#421

https://pulp.plan.io/issues/5693
closes #5693
This was done by an audit against master for pulp_rpm, pulp_container,
pulp_file, pulp_deb, pulp-certguard, and pulp_cookbook.

pulp_container will require a small update replacing its CharInFilter
usage.

https://pulp.plan.io/issues/5693
closes #5693
bmbouter added a commit to bmbouter/pulp-2to3-migration that referenced this pull request Dec 2, 2019
pulpcore changed the name of `Model` to `BaseModel`. This PR updates
pulp-2to3-migration to do the same.

Required PR: pulp/pulpcore#421

https://pulp.plan.io/issues/5693
closes #5693
Copy link
Member

@goosemania goosemania left a comment

Choose a reason for hiding this comment

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

Thank you! :octocat:

@bmbouter bmbouter merged commit b94abce into pulp:master Dec 3, 2019
@bmbouter bmbouter deleted the 5693 branch December 3, 2019 14:56
goosemania pushed a commit to goosemania/pulp_rpm that referenced this pull request Dec 10, 2019
pulpcore changed the name of `Model` to `BaseModel`. This PR updated
pulp_rpm to do the same.

Required PR: pulp/pulpcore#421

https://pulp.plan.io/issues/5693
closes #5693

(cherry picked from commit 6692804)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants