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