Skip to content

feat: create PDP API and client with matching interface#166

Closed
frrist wants to merge 1 commit intomainfrom
frrist/feat/pdp-api
Closed

feat: create PDP API and client with matching interface#166
frrist wants to merge 1 commit intomainfrom
frrist/feat/pdp-api

Conversation

@frrist
Copy link
Copy Markdown
Member

@frrist frrist commented Aug 1, 2025

  • client remains unchanged from Curio implementation
  • client and api share a type system and staify same PDP interface
  • allows client and api to be used interchangable, setting things up for single process where we optionally replace the client with the API direclty

The architecture is:

  ┌─────────────────┐
  │   HTTP Client   │
  └─────────────────┘
           │
           │ HTTP
           ↓
  ┌─────────────────┐    ┌─────────────────┐
  │   HTTP Server   │    │    API          │ ← shared business logic, can be used in process
  └─────────────────┘    └─────────────────┘
           │ uses                 │
           └──────────┬───────────┘
                      ↓
              ┌─────────────────┐
              │  PDPService     │
              └─────────────────┘

This allow alternative implementations to simply take a dependency on the API and run everything in one process.
The server simply wraps the API, interactions then go through the client. Since the Client and API both
implement the PDP interface they may be used interchangeably.
TODO: GetPieceURL is complicated as the method assumes there is an endpoint to join the piece on.

  • when operating as two process this works as it expected, the client uses the endpoint it's connected
    to to create a valid URL reference
  • when running as a single process this is a bit complicated, implementation now provides the API with an endpoint URL
    corresponding to the service its operating over

- client remains unchanged from Curio implementation
- client and api share a type system and staify same PDP interface
- allows client and api to be used interchangable, setting things up for
  single process where we optionally replace the client with the API
  direclty
Copy link
Copy Markdown
Member

@hannahhoward hannahhoward left a comment

Choose a reason for hiding this comment

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

ok, I get this now but I would recommend a bit of adjustment to clarify

as I understand it:

PDP = the general API contract
API = the implementation of that contract using the PDPService
HttpClient/HttpServer = both sides of a wire protocol to enable a PDP API contract that operates on a remote server
PDPService = the thing that does most of the logic when we run it ourselves

Some of the names then maybe could be better. I wonder if PDP should be called API, and API should be something like LocalAPI, while client would be RemoteAPI... and then the server is APIServer... just gets the fact that the two implement the same thing very clear. But I'm just spitballing none of these are set in stone.

I am slightly curious about what PDPService is vs the API. It doesn't look like there's a lot of actual code in the API itself when it's not over the wire, and some of it looks like largely type conversion code.

I don't love the inclusion of HTTP status codes in the errors in the contract. Seems like a mixing of concerns. I'd rather have custom error types (or at least categories), and then have the HTTP client/server no how to serialize them to HTTP responses.

further, I would say that unless this is a breaking change, which I don't believe it is, I think you should simply replace the API directory with your work here, rather than make an apiv2 (given that the api is essentially the same as before for the client?)

If I were to make a summary recommendation, I might say:

PDP interface -> API interface
Modify PDPService it to match the interface contract, no seperate API
Client -> HTTPAPIClient
Server -> HTTPAPIServer
define custom error types as needed for the API contract, have HTTPClient/HTTPServer figure out how to serialize/deserialize to http.

Comment thread pkg/pdp/apiv2/api.go
return nil
}

type wrapper struct {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe io.NoopCloser will give you this functionality

@frrist
Copy link
Copy Markdown
Member Author

frrist commented Aug 5, 2025

in favor of #169

@frrist frrist closed this Aug 5, 2025
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