/collection_versions/all/ endpoint is now streamed #562
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. |
|
Is it correct that the /collection_versions/all/ no longer accepts a page option? If so, we probably need a removal changelog entry. |
Pretty sure it never accepted a page option. This endpoint is suppose to be unpaginated, but the code still had remnants of the paginated code, probably from copying and pasting. |
|
@rochacbruno Still doing some testing to see if I can get it even faster. With this change I no longer get timed-out when accessing /collection_versions/all/ with 3000 collection_versions. Going to test it with 6000 collection versions and check that it still works, but I'm feeling that this might be the best current solution we can go with. Here are some results using django-silk and a distribution with 94 collection versions. Current One other thing is that the streamed response doesn't use DRF's fancy styling, it looks pretty bad in the browser. Maybe I could fix this, but I don't think it's such a big deal. |
| @@ -6,10 +6,12 @@ | |||
| from django.db.models import F | |||
| from django.db.models.expressions import Window | |||
| from django.db.models.functions.window import FirstValue | |||
| from django.http import StreamingHttpResponse | |||
pulp_ansible/app/galaxy/v3/views.py
Outdated
| cvs_template_string = "[{{ versions|map('tojson')|join(',') }}]" | ||
| cvs_template = Template(cvs_template_string) | ||
| serialized_map = map(lambda x: self.get_serializer(x, context=context).data, queryset) | ||
| streamed = map(lambda x: x.encode("utf-8"), cvs_template.stream(versions=serialized_map)) | ||
| return StreamingHttpResponse(streamed) |
There was a problem hiding this comment.
should we take these to UnpaginatedCollectionViewSet also? Or the timeout only happens here?
gerrod3
force-pushed
the
streamed_metadata
branch
from
4ad0f80
to
396ea84
Compare
gerrod3
force-pushed
the
streamed_metadata
branch
3 times, most recently
from
84aef9a
to
753a820
Compare
gerrod3
force-pushed
the
streamed_metadata
branch
from
d79854f
to
443ab28
Compare
|
@fao89 Should I tag this PR with the original issue for the unpaginated endpoint timeout https://pulp.plan.io/issues/8439? |
I think so! |
gerrod3
force-pushed
the
streamed_metadata
branch
2 times, most recently
from
ed3d0cd
to
8f79a04
Compare
gerrod3
force-pushed
the
streamed_metadata
branch
from
8f79a04
to
5621872
Compare
There was a problem hiding this comment.
This is really nice! Great job!
I believe we still need publications for 2 reasons:
- to resolve "the deprecated field issues"
- pre-computing something this large once and saving it is more efficient and will speed up the sync too. This is more of a nice-to-have really.
This is super fast I think. Potentially fast enough to rethink when or if we will need publications.