#225 Remove exchange id from http submit urls #243
Conversation
diehuxx
requested review from
michaelneale,
mistermoe,
jiyoontbd,
phoebe-lew and
KendallWeihe
as code owners
diehuxx
changed the title
| @@ -281,7 +281,7 @@ An exchange is a series of linked tbDEX messages between Alice and a PFI for a s | |||
| Submits an RFQ (Request For Quote). Alice is asking the PFI to provide a Quote so she can evaluate it. | |||
|
|
|||
| ### Endpoint | |||
| `POST /exchanges/:exchange_id` | |||
| `POST /exchanges` | |||
There was a problem hiding this comment.
is this not POST /exchanges/rfq because an RFQ creates an exchange?
There was a problem hiding this comment.
It was POST /exchanges/rfq originally but we dropped the /rfq in this spec change where we modified the request body of that endpoint to take a callback url in the replyTo field.
I'm not strongly opinionated about keeping or dropping /rfq, but I don't want to flip-flop on it more than twice.
There was a problem hiding this comment.
we're making an assumption that RFQ is always the first message to kick off an exchange so that's why we dropped the /rfq bit in the previous spec change - however my eye does twitch a little seeing that this is the only endpoint that does not specify the message kind like POST /exchanges/... in the uri.
| @@ -371,7 +371,7 @@ True | |||
| Submits the Order. Alice wants to accept the Quote and execute the transaction. | |||
|
|
|||
| ### Endpoint | |||
| `POST /exchanges/:exchange_id/order` | |||
| `POST /exchanges/order` | |||
There was a problem hiding this comment.
given that POST /exchanges creates a new exchange using an RFQ without the path pattern followed for subsequent endpoints like this one, would it make sense to use PUT instead of POST given that the exchange already exists?
something feels slightly awkward about the proposed path but maybe i'm the only one feeling that way in which case ignore.
Chad says
The choice between POST and PUT in HTTP is guided by idempotency and resource creation conventions.
POST is used for creating resources or when the operation does not need to be idempotent, meaning the same request can result in different outcomes. It's commonly used for actions like submitting form data.
PUT, on the other hand, is idempotent and used for updating resources where the outcome of multiple identical requests is the same, typically replacing a resource at a specific URL. This distinction helps ensure appropriate behavior and efficiency in web applications and API design.
based on ^ POST makes sense
There was a problem hiding this comment.
Agreed, POST seems correct for both POST /exchanges and POST /exchanges/order
There was a problem hiding this comment.
something feels slightly awkward about the proposed path but maybe i'm the only one feeling that way in which case ignore.
I'm def open to other ideas, or if this proposal just doesn't feel right, it's no biggie if we reject this one.
There was a problem hiding this comment.
I added some thoughts here #225
There was a problem hiding this comment.
LGTM, also discussed briefly with @mistermoe and @jiyoontbd that removing /rfq makes sense because of the inclusion of the replyTo field.
POST /exchange/kind for remaining message types also makes sense
|
Closing since we landed on a different proposal |
Addresses #225
There is no reason to put the exchange id in the URL because the submitted message will always contain the exchange id