Re: [Pulp-dev] Single-Table Content API Changes, Performance Discussion

[Daniel Alley]

I'm not necessarily against this but I'll recap some points I made on IRC:

The burden of knowing where to go to get that information would be pushed
off onto the API user.  If we're not returning the URL, then anyone using
the API must know that they need to query /pulp/api/v3/content/file/files/
(and likewise for every other content type), and that they need to use a
filter for repository_version=... or repository_version_added=... and so so
on.

I'm not sure how that would work, how that knowledge would be provided or
if it's something that can be hardcoded into the bindings.  If you think
that's possible, then I'm open to it.



On Thu, Dec 6, 2018 at 4:53 PM Dennis Kliban  wrote:

> On Tue, Nov 20, 2018 at 12:31 PM Dennis Kliban  wrote:
>
>> On Mon, Nov 19, 2018 at 6:20 PM Daniel Alley  wrote:
>>
>>> Some of the API changes that are required by single-table-content would
>>> be beneficial even if we didn't go forwards with the modelling changes.
>>> For instance, currently we have single endpoints for each of
>>> repository_version/.../content/,  .../added_content/, and
>>> .../removed_content/ which mix content of all types together.  This makes
>>> it impossible for clients to expect the data returned to expect any
>>> particular schema.  What the single-table-content does is to provide
>>> separate query urls for each content type present in the repository
>>> version, which I believe is a usability win for us, and it's something we
>>> could implement without using any of the modelling changes.
>>>
>>>
>> The current behavior of the 'content' APIs is already causing a problem
>> for our OpenAPI 2.0 schema. OpenAPI 2.0 does not support polymorphic
>> responses. We are currently tracking problem with a bug[0]. The only way to
>> resolve this problem is to provide APIs that return heterogeneous types.
>>
>> [0] https://pulp.plan.io/issues/4052
>>
>>
>>> Besides being a general update, I'd like to start a discussion to
>>> understand:  is changing the Pulp 3 API so that it's organized around
>>> content type URLs OK with everyone? This resolves the usability issues of
>>> returning mixed types. Are there any downsides with this approach?
>>>
>>> To clarify what I mean on that last point -- by "content type URLs" I
>>> mean that where you currently get back the url "
>>> /pulp/api/v3/repository_version/.../content/" under the "_content"
>>> field on a repoversion, you would instead get back something like
>>>
>>> { "pulp_file.filecontent":
>>> "/pulp/api/v3/content/file/files/?repository_version=.. }
>>>
>>
>> I am +1 to making this change to our REST API.
>>
>
> Thank you Daniel for putting together the patches[0,1] to make these
> changes possible. I've had a chance to try out the Python bindings. When
> using the bindings, I discovered that I could not do anything with the URLs
> returned for each content type added or removed. Making the request to
> those URLs requires making a call that looks like this:
>
>
> api.content_file_files_list(repository_version_added=repository_version_1.href)
>
> What if instead the API returned the number of each content type added or
> removed. So a repository version response would look like:
>
> {'base_version': None,
>  'content_added': {'pulp_file.file': 4},
>  'content_removed': {'pulp_file.file': 1},
>  'content_summary': {'pulp_file.file': '3'},
>  'created': datetime.datetime(2018, 12, 5, 23, 34, 26, 948749,
> tzinfo=tzlocal()),
>  'href': '/pulp/api/v3/repositories/4/versions/1/',
>  'number': 1}
>
> Thoughts?
>
> [0] https://github.com/pulp/pulp/pull/3774
> [1] https://github.com/pulp/pulp_file/pull/133
>
>
>>
>>>
>>> ___
>>> Pulp-dev mailing list
>>> Pulp-dev@redhat.com
>>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>>
>>
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

Re: [Pulp-dev] Single-Table Content API Changes, Performance Discussion

[Dennis Kliban]

On Tue, Nov 20, 2018 at 12:31 PM Dennis Kliban  wrote:

> On Mon, Nov 19, 2018 at 6:20 PM Daniel Alley  wrote:
>
>> Some of the API changes that are required by single-table-content would
>> be beneficial even if we didn't go forwards with the modelling changes.
>> For instance, currently we have single endpoints for each of
>> repository_version/.../content/,  .../added_content/, and
>> .../removed_content/ which mix content of all types together.  This makes
>> it impossible for clients to expect the data returned to expect any
>> particular schema.  What the single-table-content does is to provide
>> separate query urls for each content type present in the repository
>> version, which I believe is a usability win for us, and it's something we
>> could implement without using any of the modelling changes.
>>
>>
> The current behavior of the 'content' APIs is already causing a problem
> for our OpenAPI 2.0 schema. OpenAPI 2.0 does not support polymorphic
> responses. We are currently tracking problem with a bug[0]. The only way to
> resolve this problem is to provide APIs that return heterogeneous types.
>
> [0] https://pulp.plan.io/issues/4052
>
>
>> Besides being a general update, I'd like to start a discussion to
>> understand:  is changing the Pulp 3 API so that it's organized around
>> content type URLs OK with everyone? This resolves the usability issues of
>> returning mixed types. Are there any downsides with this approach?
>>
>> To clarify what I mean on that last point -- by "content type URLs" I
>> mean that where you currently get back the url "
>> /pulp/api/v3/repository_version/.../content/" under the "_content" field
>> on a repoversion, you would instead get back something like
>>
>> { "pulp_file.filecontent":
>> "/pulp/api/v3/content/file/files/?repository_version=.. }
>>
>
> I am +1 to making this change to our REST API.
>

Thank you Daniel for putting together the patches[0,1] to make these
changes possible. I've had a chance to try out the Python bindings. When
using the bindings, I discovered that I could not do anything with the URLs
returned for each content type added or removed. Making the request to
those URLs requires making a call that looks like this:

api.content_file_files_list(repository_version_added=repository_version_1.href)

What if instead the API returned the number of each content type added or
removed. So a repository version response would look like:

{'base_version': None,
 'content_added': {'pulp_file.file': 4},
 'content_removed': {'pulp_file.file': 1},
 'content_summary': {'pulp_file.file': '3'},
 'created': datetime.datetime(2018, 12, 5, 23, 34, 26, 948749,
tzinfo=tzlocal()),
 'href': '/pulp/api/v3/repositories/4/versions/1/',
 'number': 1}

Thoughts?

[0] https://github.com/pulp/pulp/pull/3774
[1] https://github.com/pulp/pulp_file/pull/133


>
>>
>> ___
>> Pulp-dev mailing list
>> Pulp-dev@redhat.com
>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>
>
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev

Re: [Pulp-dev] Proposal: merge the content-app & streamer

[Brian Bouterse]

I've heard a lot of positive feedback on this idea and no objections. I
made this story tracking this threads proposal specifically [0] and I made
it a subtask of the lazy epic [1]. My next step to finish the streamer is
to work on [0] so I've taken that as assigned. I'll reply to the list when
the next round of PRs are posted and passing in Travis. Hopefully that
should complete most of the work for the lazy epic [1].

[0]: https://pulp.plan.io/issues/4239
[1]: https://pulp.plan.io/issues/3693

On Wed, Dec 5, 2018 at 4:07 PM Dennis Kliban  wrote:

> Since no one is objecting, I'd like to see the PR[0] from @bmbouter
> finished up and merged soon. I am updating the pass-through cache story[1]
> to get it ready for implementation. I would like to remove all mention of
> separate content app and streamer from the description.
>
>
> [0] https://github.com/pulp/pulp/pull/3779
> [1] https://pulp.plan.io/issues/3894
>
> On Tue, Dec 4, 2018 at 1:54 PM Tatiana Tereshchenko 
> wrote:
>
>> +1 to merge
>> +1 to have clear docs for plugin writers how to create their own content
>> app
>>
>> On Mon, Dec 3, 2018 at 11:25 PM Dennis Kliban  wrote:
>>
>>> It was pointed out on IRC that plugins that have to supply their own
>>> content app (such as docker) currently need to supply 2 implementations of
>>> it in order to support on-demand use cases. One using django and another
>>> using aiohttp.
>>>
>>> We should not burden plugin writers in such a way. We really have to
>>> make the proposed change.
>>>
>>> On Mon, Dec 3, 2018 at 3:24 PM Dana Walker  wrote:
>>>
 In light of the efficiency gains and lack of significant drawbacks, I'm
 +1 on this proposal.

 --Dana


 Dana Walker

 Associate Software Engineer

 Red Hat

 
 


 On Mon, Dec 3, 2018 at 2:40 PM Dennis Kliban 
 wrote:

> I like the idea of combining the two applications for all the reasons
> already outlined on this thread. The user experience is going to be
> simplified by this change. However, I want to point out that it will also
> alter the plugin writer experience. Plugin writers that want to have their
> own content app will now need to provide it as a plugin for the content 
> app
> (which is not a Django project). We should be able to clearly document 
> this
> for plugin writers. pulp_docker plugin will need to adopt this change. For
> that reason I'd like us to make a decision on this soon.
>
> On Fri, Nov 30, 2018 at 4:59 PM Jeff Ortel  wrote:
>
>> *BACKGROUND*
>>
>> The pulp3 content app and the streamer (in-progress) currently have a
>> lot of duplicate code and functionality.  At the very least, I think 
>> there
>> is a opportunity to refactor both and share code.  But, this would leave 
>> us
>> with two components with significant overlap in functionality.
>>
>> The functionality exclusive to the content-app:
>>   - Optionally delegate file serving to a web server. (Eg:
>> mod_xsendfile).
>>   - Optional redirect to the streamer.
>>
>> The functionality exclusive to the streamer:
>>   - Using the Remote & RemoteArtifact to download the file and stream
>> on demand.
>>
>> Not much difference which raises the question: "Why do we have
>> both?"  I think the answer may be that we don't.
>>
>> *PROPOSAL*
>>
>> Let's pull the content-app out and merge it with the streamer.  The
>> new content (app) would have *streamer* architecture &
>> functionality.  When a requested artifact has not been downloaded, it 
>> would
>> download/streamed instead of REDIRECT.  This does mean that deployments 
>> and
>> development environments would need to run an additional service to serve
>> content.  The /pulp/content endpoint would be on a different port than 
>> the
>> API.  I see this separation as a healthy thing.  There is significant
>> efficiency to be gained as well.  Let's start with eliminating the
>> REDIRECTs.  Cutting the GET requests in half is a win for both the 
>> client,
>> the network and the Pulp web stack.  Next is database queries.  Since 
>> both
>> applications needed to perform many of the same queries, combining the
>> applications will roughly cut them in half as well.  Since the streamer 
>> is
>> based on asyncio and so would the merged app.
>>
>> There are probably lots of other pros/cons I have not considered but
>> it seems relatively straight forward.
>>
>> I'm thinking the new content app/service would be named:
>> *pulp-content*.
>>
>> Thoughts?
>>
>> ___
>> Pulp-dev mailing list
>> Pulp-dev@redhat.com
>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>
> 

[Pulp-dev] Please start using pulp/pulpcore-plugin

[David Davis]

Just a heads up, the repo for pulpcore-plugin is at
https://github.com/pulp/pulpcore-plugin. Please start using that repo
instead of https://github.com/pulp/pulp/tree/master/plugin. The plugin code
in pulp/pulp will be removed by Friday.

I’ve opened PRs against the devel repo[0] and ansible installer[1] to use
the new pulpcore-plugin repository.

[0] https://github.com/pulp/devel/pull/200
[1] https://github.com/pulp/ansible-pulp3/pull/52

David
___
Pulp-dev mailing list
Pulp-dev@redhat.com
https://www.redhat.com/mailman/listinfo/pulp-dev