Skip to content

Conversation

@haydenbaker
Copy link
Contributor

@haydenbaker haydenbaker commented Mar 18, 2025

Description of changes:

  • Adds the client protocol implementations
  • Several TODOs, some not necessarily needed and some are blocked by other work (e.g. endpoint properties)
  • Codegen will use the protocol when setting up the client request pipeline
    • for restjson, this could hypothetically look like:
class Weather:

    pipeline: RequestPipeline[HTTPRequest, HTTPResponse] = RequestPipeline(RestJsonClientProtocol(), ...)

    async def get_city(...) -> GetCityOutput:
        ...
        call = ClientCall(...)
        result = pipeline(call)
        ...
 

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@haydenbaker haydenbaker requested a review from a team as a code owner March 18, 2025 05:39
@haydenbaker haydenbaker force-pushed the haydenbaker/add-protocols branch from 4b56b21 to 5eafa15 Compare March 18, 2025 05:50
Comment on lines 29 to 41
for val in self.document_value["http"]:
assert isinstance(val, str)
self.http.add(val)

if vals := self.document_value.get("eventStreamHttp") is None:
object.__setattr__(self, "eventStreamHttp", self.http)
else:
# check that eventStreamHttp is a subset of http
assert isinstance(vals, Sequence)
for val in self.document_value["eventStreamHttp"]:
assert val in self.http
assert isinstance(val, str)
self.eventStreamHttp.add(val)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have validators to validate this kind of constraint model-side, but this would be good time to discuss whether we do any kind of validation in our trait definitions here. Thoughts?

@haydenbaker haydenbaker force-pushed the haydenbaker/add-protocols branch from fa3d584 to 4cc6a07 Compare March 19, 2025 05:14
@haydenbaker haydenbaker force-pushed the haydenbaker/add-protocols branch from 72d312f to 17eb3b2 Compare March 19, 2025 05:33
@haydenbaker haydenbaker force-pushed the haydenbaker/add-protocols branch from 01d3d53 to 9882b57 Compare March 19, 2025 21:05
@haydenbaker haydenbaker force-pushed the haydenbaker/add-protocols branch from 9882b57 to 3ac5a2f Compare March 19, 2025 21:18
@haydenbaker haydenbaker force-pushed the haydenbaker/add-protocols branch from 3ac5a2f to 4b6ba2c Compare March 19, 2025 21:22
JordonPhillips
JordonPhillips previously approved these changes Mar 19, 2025
uri_builder.port = uri.port
if uri.path:
uri_builder.path = os.path.join(uri.path, uri_builder.path or "")
# TODO: merge headers from the endpoint properties bag
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'll be under headers in the properties bag. Not necessary right now though.

@haydenbaker haydenbaker merged commit 072adf4 into develop Mar 19, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants