-
Notifications
You must be signed in to change notification settings - Fork 964
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
Add hypermedia API to replace XML-RPC and simple #4078
Conversation
Adds a new API that covers the usage of the XMP-RPC and the simple api. #284 This work is intended as a proof of concept for how a hypermedia API could be implemented, setting up the patterns that can be extended to cover the rest of the API. The new API introduces pagination to reduce the load for list views. Serializers are used to increase maintainability and code reuse. Some filtering is added to meet the use cases of XML-RPC. Many thanks to @werwty for hacking out an initial implementation which has been squashed. Introduces new dependencies: apispec==0.37.0 : Used to generate an api spec at /api/ marshmallow==3.0.0b10 : Used to serialize responses PyYAML==3.12 : Dependency of apispec All new endpoints are added to a new domain, "sandbox". Note: Locally, all subdomains were treated just like the actual domain so I was unable to make the subdomain works as expected. I followed the pattern that forklift uses, and guessed how it should work.
@asmacdo Awesome to see the first iteration of a next generation API coming together. Thanks for your contributions during sprints. We can still likely roll this out under the new subdomain, but do require 100% test coverage for merges. |
@@ -36,6 +38,7 @@ pyramid_retry>=0.3 | |||
pyramid_rpc>=0.7 | |||
pyramid_services | |||
pyramid_tm>=0.12 | |||
PyYaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a dependency of apispec, it shouldn’t be in the in
file.
digests = fields.Method("get_digests") | ||
size = fields.Int() | ||
upload_time = fields.Function( | ||
lambda obj: obj.upload_time.strftime("%Y-%m-%dT%H:%M:%S") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No timezone?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive-by comment: You can use fields.DateTime
here. If you need a specific format, you can pass format
:
import datetime as dt
import pytz
from marshmallow import Schema, fields
class File(Schema):
upload_time = fields.DateTime(format="%Y-%m-%dT%H:%M:%S")
schema = File()
timezone_aware_dt = dt.datetime.utcnow()
timezone_aware_dt.replace(tzinfo=pytz.UTC)
print(schema.dump({'upload_time': timezone_aware_dt}))
# {'upload_time': '2018-06-01T13:29:25'}
If you don't pass a format, the default behavior is to format as ISO8601.
import datetime as dt
import pytz
from marshmallow import Schema, fields
class File(Schema):
upload_time = fields.DateTime()
schema = File()
timezone_aware_dt = dt.datetime.utcnow()
timezone_aware_dt.replace(tzinfo=pytz.UTC)
print(schema.dump({'upload_time': timezone_aware_dt}))
# {'upload_time': '2018-06-01T13:31:08.732522+00:00'}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@merwok, we left out the timezone because that's what the JSON api does. However, the XML-RPC api does include a tz. I'll add tz to this field on the next push.
filename = fields.Str() | ||
packagetype = fields.Str() | ||
python_version = fields.Str() | ||
has_sig = fields.Bool(attribute="has_signature") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change the name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
has_sig
is what the JSON API uses, so I started there. I prefer to keep the names consistent with the db where possible, I'll change it to has_signature.
|
||
class Project(Schema): | ||
url = fields.Method("get_detail_url") | ||
releases_url = fields.Method("get_releases_url") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drive-by comment: Since serializing URLs is a common usage, you might want to create a custom field. It could look something like
class Project(Schema):
releases_url = RouteURL("api.views.projects.releases",
name="<normalized_name>")
For an example of how this might be implemented, check out flask-marshmallow's equivalent: https://github.com/marshmallow-code/flask-marshmallow/blob/7e594dc89ba853946ef1593e9ac19850a5ec9d2a/flask_marshmallow/fields.py#L39
I haven't yet, sorry, but it's on my list. |
Following discussion with team, the xmlrpc api is not deprecated today. It will not disappear soon. Also, as: - parsing the legacy html api [1] is considered bad practice - discussions exist to create equivalent apis to their deprecated/legacy apis [1] [2] We chose to implement the xmlrpc one. [1] https://warehouse.readthedocs.io/api-reference/legacy/#simple-project-api [2] pypi/warehouse#284 [3] pypi/warehouse#4078 Related T422
@di Do you think you'll have time to review this during August? |
This code has rotted a bit, so I'm closing. |
The working plan and reasoning is documented on this etherpad, comments and additions are encouraged: https://pad.sfconservancy.org/p/hypermedia_api_design
This work is based on the discussion from:
#284
Introduces new dependencies:
All new endpoints are added to a new subdomain, "api".