Good catch, Youngrae! I consider this to be a bug fix, so I do not think we need to worry about compatibility issues.
On Thu, Nov 5, 2026 at 8:39 AM Dmitri Bourlatchkov via dev <[email protected]> wrote: > > Hi JB, Youngrae, > > I totally agree that this change is correct and it is not an backward > compatibility issue. > > Nice catch on adding the 400 response to the API spec! > > I approved PR [5909] in GH. > > As usual for API-level changes, let's give it some more time for reviews. > Merging late on Nov 6 seems reasonable to me, if there are no > further comments. > > Cheers, > Dmitri. > > [5909] https://github.com/apache/polaris/pull/5909 > > On Thu, Nov 5, 2026 at 4:23 AM youngrae kim <[email protected]> wrote: > > > Hi JB, > > > > Thanks for the review and the +1. > > > > Good point on the 400 response. > > I have added it to createCatalog in the Management API spec in the latest > > commit of the PR: the response is declared with the description > > "The request is invalid, for example an unsupported or incomplete storage > > or connection configuration". > > No model or generated interface changes, the generated code compiles as > > before. > > > > If there are no further objections I will leave the PR for the committers > > to merge. > > > > Best regards, > > Youngrae Kim > > > > 2026년 11월 5일 (목) 오후 5:31, Jean-Baptiste Onofré <[email protected]>님이 작성: > > > > > Hi Youngrae, > > > > > > Good call from Dmitri asking you to bring the discussion on the list :) > > > > > > I'm +1 on the change. Returning a 500 for invalid client input is a bug > > > imho, not part of the API contract, and no client should rely on getting > > a > > > 500 for a malformed request. I don't see any compatibility concern > > > switching to 400. Furthermore, ConnectionConfigInfo is still > > experimental, > > > so no problem to change it ;) > > > > > > I also agree with documenting authenticationParameters in the description > > > rather than adding it to required. That way the models don't change for > > > existing clients. > > > > > > I have just one small thing (not blocker): createCatalog in the > > Management > > > API doesn't declare a 400 response at all. The endpoint already returns > > 400 > > > for invalid storage config, and with your PR it will also do so for > > invalid > > > connection config, so it would be good to document it. > > > > > > I'm fine to merge if no objections here. > > > > > > Regards > > > JB > > > > > > On Thu, Nov 5, 2026 at 6:12 AM youngrae kim <[email protected]> > > > wrote: > > > > > > > Hi all, > > > > > > > > Dmitri asked me to bring PR #5909 [1] to the list because it changes > > the > > > > HTTP status code of two > > > > Management API error responses. > > > > > > > > Today, creating an EXTERNAL catalog with an invalid > > connectionConfigInfo > > > > fails with > > > > 500 Internal Server Error in two cases, although both are client input > > > > errors: > > > > > > > > 1. a connectionType or authenticationType that is not in > > > > SUPPORTED_CATALOG_CONNECTION_TYPES / > > > > SUPPORTED_EXTERNAL_CATALOG_AUTHENTICATION_TYPES (an > > > > IllegalStateException escapes), and > > > > 2. a request without authenticationParameters (a NullPointerException > > > > escapes; the spec only > > > > marks connectionType as required). > > > > > > > > The PR makes both return 400 Bad Request. The messages for case 1 are > > > > unchanged; case 2 gets > > > > "authenticationParameters is required for an external catalog". The > > spec > > > > description of > > > > ConnectionConfigInfo now states that authenticationParameters is > > > required. > > > > Valid requests are > > > > unaffected, and nothing else in the error handling changes. Issue: > > #5908 > > > > [2]. > > > > > > > > The PR has one approval (Dmitri). If nobody objects within a few days I > > > > would ask for it to be > > > > merged as is; happy to adjust if there are concerns about the code or > > the > > > > message. > > > > > > > > [1] https://github.com/apache/polaris/pull/5909 > > > > [2] https://github.com/apache/polaris/issues/5908 > > > > > > > > Best regards, > > > > Youngrae Kim > > > > > > > > >
