Project

Profile

Help

Task #3102

Task #2868: Platform support for publishing.

Make Distribution a top level resource in the API.

Added by jortel@redhat.com almost 2 years ago. Updated 6 months ago.

Status:
MODIFIED
Priority:
Normal
Category:
-
Sprint/Milestone:
Start date:
Due date:
% Done:

100%

Platform Release:
Blocks Release:
Backwards Incompatible:
No
Groomed:
Yes
Sprint Candidate:
Yes
Tags:
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:
Sprint 29

Description

This task is to:
  1. Move distributions to the top level resource (no longer owned by a publisher).
  2. update the uniqueness constrains for distribution name (no longer unique together with publisher name)
  3. remove auto_updated field
  4. rename publisher_id to publisher, this field should be optional. (If this is set, it implies that the distribution will be automatically updated by that publisher.)
  5. Update the publish task to make sure publishers can update related distributions when a new publication is created.

Background

During a discussion with Austin to resolve a problem implementing #3033, an interested question was raised -
"Why do Distributions needs to be owned by Publishers?" This question came up when considering a solution to
a DRF difficulty related to both Publications and Distributions being nested under publisher/ AND related to
each other. The idea being considered was to move Distributions to a top level resource. Here are the benefits:

1. Resolves current DRF nesting issue w/ #3033. (This is minor).
2. A distribution could be updated to reference any publication. This is more flexible.
3. Since Distribution.base_path is unique across all repositories/publishers, it might be more intuitive to be
a top level resource?

Currently, the Distribution.publisher represents a parent-child relationship the mainly exists to support
automatic distribution. When the publisher creates a new publication, it is automatically associated to any
of the publisher's distributions marked as auto_updated=True.

There are two challenges to moving the Distribution to a top-level resources.

1. The distribution name is currently unique by (publisher, name).
2. This would break automatic distribution as currently implemented.

Here are a few options to resolving these challenges:

1. The name could be unique across all distributions. This seems reasonable.
2. Redesign automatic distribution. (see proposal below).
3. Reconsider automatic distribution.

Proposal:

The use case for automatic distribution is similar to automatic publishing. The user has updated a
repository; has published it; and now wants to consume content. This could be done by making 3 API calls: 1
sync; 2 publish; 3 update-a-distribution. But, based on pulp2, users want to do this with 1 API call.

New ER diagram:


Publisher <---* Publication
   |                ^ (0,1)
   |                |
   |                |
   v*               |
Distribution --------


Checklist


Related issues

Blocks Pulp - Task #3033: Add Publication API Endpoint. MODIFIED Actions

Associated revisions

History

#1 Updated by jortel@redhat.com almost 2 years ago

  • Blocks Task #3033: Add Publication API Endpoint. added

#2 Updated by jortel@redhat.com almost 2 years ago

  • Parent task set to #2868

#3 Updated by jortel@redhat.com almost 2 years ago

  • Description updated (diff)

#4 Updated by jortel@redhat.com almost 2 years ago

  • Description updated (diff)

#5 Updated by jortel@redhat.com almost 2 years ago

Updated to retain support for automatic distribution to multiple distributions (base paths).

#6 Updated by amacdona@redhat.com almost 2 years ago

1) Are we in agreement that a distribution's url is `api/v2/distributions/<name>/` making Distrubition.name unique (no longer unique together with repositories)?

2) We should rename the `publisher_id` field to be more descriptive. Does "subscribed_to" seems right to me. Google's definition of subscribe: "arrange to receive something regularly, typically a publication"

#7 Updated by jortel@redhat.com almost 2 years ago

wrote:

1) Are we in agreement that a distribution's url is `api/v2/distributions/<name>/` making Distrubition.name unique (no longer unique together with repositories)?

Since Distribution would be a top level resource that represents a system-wide unique distribution base path (URL), I think it's appropriate for the name to also be universally unique.

2) We should rename the `publisher_id` field to be more descriptive. Does "subscribed_to" seems right to me. Google's definition of subscribe: "arrange to receive something regularly, typically a publication"

I think publisher_id is more descriptive and appropriate.

#8 Updated by bmbouter almost 2 years ago

+1 to moving this to be a top-level resource.

Is 'name' mutable? If so we can't use it in the url based on some recent discussions. I can elaborate more if need be.

Instead of 'publisher_id' it probably should be 'publisher' since the representation of that relationship won't be an ID but an actual publisher link. A field named id I would expect to contain an identifier not a full url.

#9 Updated by amacdona@redhat.com almost 2 years ago

I think it makes more sense to make "name" immutable than to use id. Whether by CLI or by REST API, users will want to use a name.

I agree that keys that are a part of urls should be immutable, but I don't think that needs to be a part of this work. Name is currently mutable, and part of the url. Instead, I'd prefer to include Distributions (and other applicable objects) in https://pulp.plan.io/issues/3101.

#10 Updated by bmbouter almost 2 years ago

I took away a different conclusion from the irc discussion about natural keys in urls. I took away that we should not strive to use natural keys in urls because they are mutable and that we shouldn't strive to have readable urls since they are really for machines anyway. Readable urls are important for SEO and those situations, but not as much for APIs (I think). Making an attribute read only to make the url prettier I don't think is a win.

Either way, we can separate these two discussions. For the purposes of this I'm +1 to making it top level resource with whatever urls you want because ultimately we're going to have to revisit our natural key usage in urls anyway.

#11 Updated by jortel@redhat.com almost 2 years ago

bmbouter wrote:

+1 to moving this to be a top-level resource.

Is 'name' mutable? If so we can't use it in the url based on some recent discussions. I can elaborate more if need be.

My position on this is the same as repository name. Distribution.name should be mutable and not used in URLs.

Instead of 'publisher_id' it probably should be 'publisher' since the representation of that relationship won't be an ID but an actual publisher link. A field named id I would expect to contain an identifier not a full url.

Understood. I was talking in ER diagramming terms not django modeling. The Distribution.publisher attribute will result in a DB column named "publisher_id". I updated the description for clarity.

#12 Updated by jortel@redhat.com almost 2 years ago

  • Description updated (diff)

#13 Updated by amacdona@redhat.com almost 2 years ago

  • Description updated (diff)

#14 Updated by amacdona@redhat.com almost 2 years ago

  • Description updated (diff)
  • Groomed changed from No to Yes
  • Sprint Candidate changed from No to Yes
  • Tags Pulp 3 added

#15 Updated by mhrivnak almost 2 years ago

  • Sprint/Milestone set to 47

#16 Updated by amacdona@redhat.com almost 2 years ago

  • Description updated (diff)

#17 Updated by jortel@redhat.com almost 2 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to jortel@redhat.com

#18 Updated by jortel@redhat.com almost 2 years ago

The DRF mixin classes maintained by pulp do not seem to support references to a nested master-detail resource outside the nested hierarchy. Austin is investigating.

#19 Updated by amacdona@redhat.com almost 2 years ago

I tracked the problem down to drf-nested-routers. It has been fixed upstream, but has not been released yet.

Depending on their release schedule, we have a couple options (1) override this function with corrected code (2) carry the dep until they release.

https://github.com/alanjds/drf-nested-routers/commit/6185da2c2e81bd452a87e87c3781b61964513873#diff-8a5d58427acdc6881c2096ebbefc0cefR79

#20 Updated by rchan almost 2 years ago

  • Sprint/Milestone changed from 47 to 48

#21 Updated by daviddavis almost 2 years ago

  • Tags Pulp 3 MVP added

#22 Updated by jortel@redhat.com almost 2 years ago

  • Status changed from ASSIGNED to MODIFIED
  • % Done changed from 0 to 100

#23 Updated by bmbouter over 1 year ago

  • Sprint set to Sprint 29

#24 Updated by bmbouter over 1 year ago

  • Sprint/Milestone deleted (48)

#25 Updated by dkliban@redhat.com over 1 year ago

  • Sprint/Milestone set to 3.0

#26 Updated by bmbouter 6 months ago

  • Tags deleted (Pulp 3, Pulp 3 MVP)

Please register to edit this issue

Also available in: Atom PDF