Thanks Amogh for starting this discussion. I agree that using 400 makes sense, especially since the server might not fully recognize the request. It’s a straightforward way to handle these situations and avoid potential misunderstanding.
Yufei On Tue, Aug 6, 2024 at 5:53 AM Xianjin YE <xian...@apache.org> wrote: > 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> 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> >>> 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 >>>> >>> >
