kbendick commented on pull request #3770:
URL: https://github.com/apache/iceberg/pull/3770#issuecomment-1003882891


   > Thanks for your review @jackye1995
   > 
   > > The big questions I have are:
   > > 
   > > 1. namespace property, can we just use `PUT namespace` for that
   > 
   > My one concern with using an unqualified `PUT namespace` (unqualified 
meaning nothing in the path representing that it's a `properties` request) is 
that backend frameworks would need to be able to handle any shape of body 
depending on what is being updated. For namespace, that would only be 
properties as far as I can tell, so this one might not be too bad. But for 
`tables` etc, that could get unwieldy and I don't love the idea of using a 
plain `PUT` for namespace and then not necessarily doing that for tables etc. 
The `PUT namespaces` would still more or less need to have the same body to 
differentiate between properties to remove and properties to upsert.
   
   > 
   > > 2. `/v1/tables/rename`, can we use `/v1/table-ops/rename` or something 
else to make the path more self-explanatory
   > 
   > I'm presently against using anything with `table-ops` in the name for now, 
only as we have had a decent amount of discussion on topics so far, but 
`table-ops` is still yet to be fully discused.
   > 
   > Can we can mentally mark it as potentially re-namable to `table-ops` or 
something if we go with that after we discuss more of the routes that involve 
`table-ops`? I feel like that's going to be a bigger discussion (using most of 
the `table-ops`).
   > 
   > I also feel that `table-ops` is _maybe_ too resource oriented vs just 
`tables`, but that's a discussion for `table-ops` in general which I'm sure 
will be soon.
   > 
   > > 3. I think we do need 5xx and 400 error definitions in the spec based on 
RFC 7231, and we can simplify the error response using a universal error schema.
   > 
   > Yes we do. I have seen them with exactly `400` and `5xx` (e.g. in the 
[Confluent Kafka REST OpenAPI 
spec](https://github.com/confluentinc/kafka-rest/blob/b0259cfae59f68d93c9c32087e1aa96efeec6a74/api/v3/openapi.yaml#L77-L78)
 I recently found). I will add those tomorrow (more or less just as is on that 
doc)!
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to