dennishuo opened a new pull request, #440:
URL: https://github.com/apache/polaris/pull/440

   The way the API model for Catalogs splits out the default-base-location as a 
required field separate from "additionalProperties" makes it easy for client 
libraries to forget to forget to "echo" an existing default-base-location when 
trying to set new properties.
   
   Since the default-base-location is required during catalog creation, it 
definitely wasn't correct to allow silently clobbering it in an update. So the 
only possibilities are:
   
   1. Throw explicit error if it's missing
   2. Treat it the same as when an UpdateCatalogRequest doesn't specify a 
StorageConfigInfo, which is to assume that the unspecified portion of the 
request should be unaltered
   
   Given the structure of the API making it easy to miss this fact, I'd say the 
most correct semantic here is to treat the same as unspecified 
StorageConfigInfo, and just leave default-base-location unchanged if it's 
missing from the UpdateCatalogRequest.
   
   # Description
   
   Please include a summary of the changes and the related issue. Please also 
include relevant motivation and context. List any dependencies that are 
required for this change.
   
   Fixes # (issue)
   
   ## Type of change
   
   Please delete options that are not relevant.
   
   - [ ] Bug fix (non-breaking change which fixes an issue)
   
   # How Has This Been Tested?
   
   Please describe the tests that you ran to verify your changes. Provide 
instructions so we can reproduce. Please also list any relevant details for 
your test configuration
   
   - [ ] Test A
   - [ ] Test B
   
   **Test Configuration**:
   * Hardware:
   * Toolchain:
   * SDK:
   
   # Checklist:
   
   Please delete options that are not relevant.
   
   - [ ] I have performed a self-review of my code
   - [ ] I have commented my code, particularly in hard-to-understand areas
   - [ ] I have made corresponding changes to the documentation
   - [ ] My changes generate no new warnings
   - [ ] If adding new functionality, I have discussed my implementation with 
the community using the linked GitHub issue
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to