Thanks Amogh for driving this discussion. I’m also +1 for 400 status code as others pointed out that the server is unable to determine the request is well formed or not.
> On Aug 6, 2024, at 05:28, Amogh Jahagirdar <2am...@gmail.com> wrote: > > I also went back and forth on 400 vs 422 but ultimately concluded that 400 is > the correct one to use here. My understanding is that 422 is meant to address > semantic issues in the request as opposed to 400 which is typically used for > invalid formatted input. > > As Dan mentioned, in this case the server is actually unable to identify if > the request was well formed, since it does not even have an understanding of > if the requirement/update is valid. If the server is not able to have a > semantic understanding of this requirement/update, then I think from a server > perspective it must be concluded that it is invalid input. > > >(Any other options considered?) > > I didn't really consider any other high level options tbh since I think they > lead to bad behaviors, but for the sake of thinking it through explicitly: > The options for handling unknown input range from ignoring it to spec'ing out > the required failure mode (this proposal). > If servers ignored the update rather than fail, then clients would think > their operations would succeed but they really didn't. If a server ignored a > requirement it couldn't understand, that could lead to correctness issues! > > As a result, I think the clearest option is to have a defined failure mode. > > Thanks, > > Amogh Jahagirdar > > On Mon, Aug 5, 2024 at 9:22 AM Daniel Weeks <dwe...@apache.org > <mailto:dwe...@apache.org>> wrote: >> I feel like this is a little bit of a gray area in terms of 400 vs 422. >> While I agree that 422 reads like the right answer just based on the >> definition of the codes, I think that it will be hard to implement and may >> not make sense in context of how the server evolves. >> >> If a server has not implemented a new update/requirement, then it would be >> seen as a bad/request (400) because the server doesn't have the ability to >> determine whether the request is well formatted or not (it knows nothing >> about the request). >> >> I would say the 422 makes sense if the server understands and can parse the >> request, but cannot complete the request, which would not be the case. It >> would also be hard for a service to determine if the request was well formed >> if it does not know about the request. >> >> I think 400 makes the most sense here. >> >> -Dan >> >> On Fri, Aug 2, 2024 at 4:28 PM Dmitri Bourlatchkov >> <dmitri.bourlatch...@dremio.com.invalid> wrote: >>> I'd suggest using error code 422 (Unprocessable Content) [1] instead. >>> >>> 400 generally means the request was not well-formed (e.g. syntax error, or >>> using invalid chars, etc.). However, here we have a situation when a client >>> actually sends a valid request, but the server is not able to properly >>> process some part of it. >>> >>> Cheers, >>> Dmitri. >>> >>> [1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/422 >>> >>> On Fri, Aug 2, 2024 at 1:15 PM Amogh Jahagirdar <2am...@gmail.com >>> <mailto:2am...@gmail.com>> wrote: >>>> Hey everyone, >>>> >>>> I'm starting this thread to discuss clarifying in the REST Spec on what >>>> servers should do >>>> if as part of a commit operation they receive an update >>>> <https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L2570C5-L2570C16> >>>> or requirement >>>> <https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L2601> >>>> that is unknown. >>>> >>>> What I am proposing is to clearly say in the spec that if a server >>>> receives an update or requirement which is unknown it must fail with a 400 >>>> error code. 400 seems the most appropriate code to be able to describe >>>> unknown input in the commit table request. Here's the PR >>>> https://github.com/apache/iceberg/pull/10848/files >>>> >>>> This is a simple way to ensure that implementations avoid correctness >>>> issues that come from ignoring unknown updates/requirements. It also >>>> enables the community to be able to add new operations without any complex >>>> versioning schemes. >>>> >>>> A more concrete example: Recently there's been work on removing inactive >>>> partition specs <https://github.com/apache/iceberg/pull/10755>from table >>>> metadata. With this proposal, any server which receives a >>>> RemovePartitionSpecUpdate and does not recognize it, must fail with a 400. >>>> >>>> Note: I will propose the specific example of RemovePartitionSpecUpdate >>>> spec change in a different thread. >>>> >>>> Thanks, >>>> >>>> Amogh Jahagirdar
