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 > > > > > >
