pulpcore with drf-spectacular #785

fao89 · 2020-07-10T22:27:19Z

Required PR: pulp/pulp_file#413
https://pulp.plan.io/issues/7108
closes #7108

Please be sure you have read our documentation on creating PRs:
https://docs.pulpproject.org/contributing/pull-request-walkthrough.html

Required PR: pulp/pulpcore#785 [noissue]

pulpbot · 2020-07-10T22:33:44Z

Attached issue: https://pulp.plan.io/issues/7108

Required PR: pulp/pulpcore#785 [noissue]

mdellweg

Great work @fao89 !

.travis/before_install.sh

mdellweg · 2020-07-16T10:08:38Z

.travis/test_bindings.rb

@@ -86,7 +86,7 @@ def upload_file_in_chunks(file_path)
          filechunk.write(chunk)
          filechunk.flush()
          actual_chunk_size = File.size(filechunk)
-          response = @uploads_api.update(upload_href, content_range(offset, offset + actual_chunk_size -1, total_size), filechunk)
+          response = @uploads_api.update(content_range(offset, offset + actual_chunk_size -1, total_size), upload_href, filechunk)


Should we use keyword arguments here?

I tried, but as I don't know ruby, I think I did something wrong and it didn't work

have you tried it like this?

Suggested change

response = @uploads_api.update(content_range(offset, offset + actual_chunk_size -1, total_size), upload_href, filechunk)

response = @uploads_api.update(content_range: content_range(offset, offset + actual_chunk_size -1, total_size), upload_href: upload_href, filechunk: filechunk)

Nope! I tried:

content_range_value = content_range(offset, offset + actual_chunk_size -1, total_size) response = @uploads_api.update(upload_href: upload_href, content_range: content_range_value, filechunk: filechunk)

I tried: c2f478a got: https://travis-ci.com/github/pulp/pulpcore/jobs/362256353#L2545
I tried: b34e182 got: https://travis-ci.com/github/pulp/pulpcore/jobs/362258414#L2544

Well, if no real rubyist wants to jump in, we should not be blocking on this.

The method doesn't accept keyword args:

def update(content_range, upload_href, file, opts = {}) # ... end

It would need to be def update(content_range:, upload_href:, file:, opts = {}).

CHANGES/7108.feature

mdellweg · 2020-07-16T10:14:47Z

pulpcore/app/urls.py

-schema_view = get_schema_view(title="Pulp API", permission_classes=[permissions.AllowAny],)
+schema_view = get_schema_view(
+    title="Pulp API", permission_classes=[permissions.AllowAny], generator_class=PulpSchemaGenerator
+)

 urlpatterns.append(url(r"^{api_root}$".format(api_root=API_ROOT), schema_view))


Do we still want that endpoint there? I think it hides part of the DRF browsable API here.

mdellweg · 2020-07-16T10:19:27Z

pulpcore/openapi/__init__.py

+        serializer = force_instance(self.get_request_serializer())
+        for field_name, field in getattr(serializer, "fields", {}).items():
+            if isinstance(field, serializers.FileField) and self.method in ("PUT", "PATCH", "POST"):
+                return ["multipart/form-data", "application/x-www-form-urlencoded"]


Maybe not in this PR, but i'd like us to experiment, whether this hack is necessary if we use the DRF content-negotiation properly.

the problem with this is python bindings get lost, it just gets the first, so I did it to ensure multipart/form-data will be the first one

I did some research here. Seems like when you want to upload any data, "multipart/form-data" is the only usable one. But as the file field is optional we should maybe supporting "application/json" as the second option.
But, this is probably out of scope for this PR.

mdellweg · 2020-07-16T10:39:46Z

From: https://swagger.io/specification/
It is RECOMMENDED that the root OpenAPI document be named: openapi.json or openapi.yaml.

Is it possible to serve the api schema as a static asset that is generated at install time?

fao89 · 2020-07-16T14:46:56Z

From: https://swagger.io/specification/
It is RECOMMENDED that the root OpenAPI document be named: openapi.json or openapi.yaml.

Is it possible to serve the api schema as a static asset that is generated at install time?

I believe so, but I didn't look at it, but skimming through drf-spectacular code, it seems it has a CLI for it

Required PR: pulp/pulpcore#785 Required PR: pulp/pulp_file#413 Required PR: pulp/pulp_rpm#1779 Required PR: pulp/pulp_container#128 [noissue]

Required PR: pulp/pulpcore#785 [noissue]

Required PR: pulp/pulpcore#785 Required PR: pulp/pulp_file#413 Required PR: pulp/pulp_rpm#1779 Required PR: pulp/pulp_container#128 [noissue]

Required PR: pulp/pulpcore#785 [noissue]

pulpcore/app/settings.py

daviddavis · 2020-07-24T17:53:04Z

CHANGES/plugin_api/7108.removal

+Replaced drf-yasg with drf-spectacular.
+- This updates the api documentation to openapi v3.
+- Plugins may require changes.
+- Methods signatures for bindings may change.


Thanks for the plugin writer changelog.

I think we still need a user facing changelog entry too. Probably include that the schema is openapi v3 and this line that the method signatures in the bindings may change.

I have a question, is it possible to have:

CHANGES/7108.removal CHANGES/plugin_api/7108.removal

because I think it is not possible to have both with .removal, if so, which one should be removal? And the other one? feature?

Required PR: pulp/pulp_file#413 https://pulp.plan.io/issues/7108 closes #7108

Required PR: pulp/pulpcore#785 [noissue] [nocoverage]

daviddavis · 2020-07-25T12:55:27Z

@dkliban I thought we agreed to wait until Monday?

daviddavis · 2020-07-26T11:36:09Z

Nevermind. I misunderstood our conversation. Sorry!

Required PR: pulp/pulpcore#785 [noissue]

fao89 force-pushed the 7108 branch from cce7e2d to 6dac1cf Compare July 10, 2020 22:31

fao89 added a commit to fao89/pulp_file that referenced this pull request Jul 10, 2020

Replace drf-yasg with drf-spectacular

0cd1525

Required PR: pulp/pulpcore#785 [noissue]

fao89 force-pushed the 7108 branch from 6dac1cf to 891fcc2 Compare July 10, 2020 22:36

fao89 changed the title ~~drf-yasg tp drf-spectacular~~ pulpcore with drf-spectacular Jul 10, 2020

fao89 force-pushed the 7108 branch 5 times, most recently from 664674e to 7392564 Compare July 13, 2020 17:29

fao89 added a commit to fao89/pulp_file that referenced this pull request Jul 13, 2020

Replace drf-yasg with drf-spectacular

96cf8ed

Required PR: pulp/pulpcore#785 [noissue]

fao89 force-pushed the 7108 branch 5 times, most recently from 0a96276 to 6cff5ed Compare July 14, 2020 15:51

fao89 added a commit to fao89/pulp_file that referenced this pull request Jul 14, 2020

Replace drf-yasg with drf-spectacular

2b34143

Required PR: pulp/pulpcore#785 [noissue]

fao89 force-pushed the 7108 branch 3 times, most recently from a7a6906 to a6bdfe0 Compare July 14, 2020 21:27

fao89 added a commit to fao89/pulp_file that referenced this pull request Jul 14, 2020

Replace drf-yasg with drf-spectacular

4ab3029

Required PR: pulp/pulpcore#785 [noissue]

fao89 marked this pull request as ready for review July 14, 2020 21:51

mdellweg reviewed Jul 16, 2020

View reviewed changes

fao89 force-pushed the 7108 branch 5 times, most recently from c2f478a to b34e182 Compare July 17, 2020 19:59

fao89 added a commit to fao89/pulp-2to3-migration that referenced this pull request Jul 22, 2020

OpenAPI v3 with drf-spectacular

7c993bb

Required PR: pulp/pulpcore#785 Required PR: pulp/pulp_file#413 Required PR: pulp/pulp_rpm#1779 Required PR: pulp/pulp_container#128 [noissue]

fao89 mentioned this pull request Jul 22, 2020

OpenAPI v3 with drf-spectacular pulp/pulp-2to3-migration#180

Merged

fao89 added a commit to fao89/pulp_deb that referenced this pull request Jul 22, 2020

OpenAPI v3 with drf-spectacular

f5af6e9

Required PR: pulp/pulpcore#785 [noissue]

fao89 mentioned this pull request Jul 22, 2020

OpenAPI v3 with drf-spectacular pulp/pulp_deb#189

Merged

fao89 added a commit to fao89/pulp-2to3-migration that referenced this pull request Jul 22, 2020

OpenAPI v3 with drf-spectacular

d0882b7

Required PR: pulp/pulpcore#785 Required PR: pulp/pulp_file#413 Required PR: pulp/pulp_rpm#1779 Required PR: pulp/pulp_container#128 [noissue]

mdellweg added a commit to mdellweg/pulp_python that referenced this pull request Jul 23, 2020

OpenAPI v3 with drf-spectacular

17a3c35

Required PR: pulp/pulpcore#785 [noissue]

mdellweg mentioned this pull request Jul 23, 2020

OpenAPI v3 with drf-spectacular pulp/pulp_python#300

Merged

mdellweg added a commit to mdellweg/pulp_gem that referenced this pull request Jul 23, 2020

OpenAPI v3 with drf-spectacular

09ea787

Required PR: pulp/pulpcore#785 [noissue]

mdellweg mentioned this pull request Jul 23, 2020

OpenAPI v3 with drf-spectacular pulp/pulp_gem#32

Merged

mdellweg added a commit to mdellweg/pulp_npm that referenced this pull request Jul 23, 2020

OpenAPI v3 with drf-spectacular

94a15a5

Required PR: pulp/pulpcore#785 [noissue]

mdellweg mentioned this pull request Jul 23, 2020

OpenAPI v3 with drf-spectacular pulp/pulp_npm#21

Merged

mdellweg added a commit to mdellweg/pulp_gem that referenced this pull request Jul 23, 2020

OpenAPI v3 with drf-spectacular

5655e00

Required PR: pulp/pulpcore#785 [noissue]

mdellweg added a commit to mdellweg/pulp_python that referenced this pull request Jul 23, 2020

OpenAPI v3 with drf-spectacular

2d0a6b6

Required PR: pulp/pulpcore#785 [noissue]

mdellweg added a commit to mdellweg/pulp_gem that referenced this pull request Jul 23, 2020

OpenAPI v3 with drf-spectacular

7fe46f9

Required PR: pulp/pulpcore#785 [noissue]

mdellweg added a commit to mdellweg/pulp_npm that referenced this pull request Jul 23, 2020

OpenAPI v3 with drf-spectacular

603a2f6

Required PR: pulp/pulpcore#785 [noissue]

mdellweg reviewed Jul 24, 2020

View reviewed changes

pulpcore/app/settings.py Outdated Show resolved Hide resolved

fao89 force-pushed the 7108 branch from 8fb85c7 to 44dfc05 Compare July 24, 2020 14:32

daviddavis reviewed Jul 24, 2020

View reviewed changes

dkliban approved these changes Jul 24, 2020

View reviewed changes

drf-yasg to drf-spectacular

c42931f

Required PR: pulp/pulp_file#413 https://pulp.plan.io/issues/7108 closes #7108

fao89 force-pushed the 7108 branch from 44dfc05 to c42931f Compare July 24, 2020 18:34

daviddavis approved these changes Jul 24, 2020

View reviewed changes

dkliban merged commit 6126111 into pulp:master Jul 24, 2020

fao89 added a commit to fao89/pulp_rpm that referenced this pull request Jul 24, 2020

OpenAPI v3 with drf-spectacular

d454ef6

Required PR: pulp/pulpcore#785 [noissue] [nocoverage]

fao89 added a commit to fao89/pulp_rpm that referenced this pull request Jul 24, 2020

OpenAPI v3 with drf-spectacular

5d4c004

Required PR: pulp/pulpcore#785 [noissue] [nocoverage]

mdellweg added a commit to mdellweg/pulp_npm that referenced this pull request Jul 27, 2020

OpenAPI v3 with drf-spectacular

1841da5

Required PR: pulp/pulpcore#785 [noissue]

fao89 mentioned this pull request Jul 29, 2020

From OpenAPI v2 to OpenAPI v3 pulp/pulp_cookbook#83

Merged

ThikaXer pushed a commit to ATIX-AG/pulp_deb that referenced this pull request Oct 5, 2020

OpenAPI v3 with drf-spectacular

0205aff

Required PR: pulp/pulpcore#785 [noissue]

pulpcore with drf-spectacular #785

pulpcore with drf-spectacular #785

fao89 commented Jul 10, 2020

pulpbot commented Jul 10, 2020

mdellweg left a comment

mdellweg Jul 16, 2020

fao89 Jul 16, 2020 •

edited

mdellweg Jul 17, 2020

fao89 Jul 17, 2020 •

edited

fao89 Jul 17, 2020

mdellweg Jul 20, 2020

daviddavis Jul 20, 2020

mdellweg Jul 16, 2020

mdellweg Jul 16, 2020

fao89 Jul 16, 2020

mdellweg Jul 20, 2020

mdellweg commented Jul 16, 2020

fao89 commented Jul 16, 2020

daviddavis Jul 24, 2020

fao89 Jul 24, 2020

daviddavis commented Jul 25, 2020

daviddavis commented Jul 26, 2020

	response = @uploads_api.update(content_range(offset, offset + actual_chunk_size -1, total_size), upload_href, filechunk)
	response = @uploads_api.update(content_range: content_range(offset, offset + actual_chunk_size -1, total_size), upload_href: upload_href, filechunk: filechunk)

pulpcore with drf-spectacular #785

pulpcore with drf-spectacular #785

Conversation

fao89 commented Jul 10, 2020

pulpbot commented Jul 10, 2020

mdellweg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fao89 Jul 16, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fao89 Jul 17, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdellweg commented Jul 16, 2020

fao89 commented Jul 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daviddavis commented Jul 25, 2020

daviddavis commented Jul 26, 2020

fao89 Jul 16, 2020 •

edited

fao89 Jul 17, 2020 •

edited