Thanks all for your feedback! Since it seems like there's general consensus and this discussion thread has been open for quite some time at this point, I will start a vote thread on this.
Thank you, Amogh Jahagirdar On Wed, Aug 7, 2024 at 7:17 PM Yufei Gu <flyrain...@gmail.com> wrote: > 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 >>>>> >>>> >>
