Hi Yun, It looks good to me, thanks !
Regards JB On Tue, Mar 4, 2025 at 8:31 PM yun zou <yunzou.colost...@gmail.com> wrote: > > Hi All, > > Thanks a lot for all the valuable feedback! > > Summary for the discussion we had during the offline meeting: > 1) For the new APIs, we will put it under a new prefix */catalog/polaris/ > to avoid any future conflict with Iceberg and indicate clearly that those > set of APIs are defined by Polaris. > 2) For existing APIs, they will remain as if today to avoid breaking any > existing users. We will bring the discussion about adding a new prefix > */catalog/iceberg for Iceberg APIs when Iceberg is ready to move to v2. > 3) In the coming release, we will add clear documentation about the APIs > that are defined by Polaris and APIs defined by Iceberg. > > Please feel free to add things if there is anything that I am missing here! > > Best Regards, > Yun > > On Mon, Mar 3, 2025 at 5:17 PM Michael Collado <collado.m...@gmail.com> > wrote: > > > -1 on changing the path for the existing table APIs. People already have > > production systems pointing to Polaris endpoints and I don't think there's > > a strong reason to force them to migrate. I think adding new prefixes for > > new functionality is fine, but adding new features shouldn't break existing > > behavior without a very strong reason. > > > > Mike > > > > On Thu, Feb 27, 2025 at 10:55 AM Robert Stupp <sn...@snazy.de> wrote: > > > > > WRT to the term "generic", I suspect that it can evolve to a more > > > generic catalog API, that could also expose functionality for Iceberg > > > tables + views and isn't limited to "generic tables" (too many different > > > meanings of "generic" there...). So not a fan of using the term > > > "generic" in a REST API (base) path. > > > > > > Although I'd prefer to keep the existing base paths, even if > > > /api/catalog isn't "prominently telling" that it's actually Iceberg > > > REST, the proposals > > > * /api/catalog/iceberg and > > > * /api/catalog/polaris > > > are probably clearer and easier for users to understand. > > > > > > So +1 on migrating Iceberg REST from /api/catalog to > > > /api/catalog/iceberg and adding /api/catalog/polaris. The then "legacy" > > > endpoint /api/catalog/v1 however should then be deprecated for removal > > > (Polaris 1.0?). > > > > > > For that API we need a (new) set of types. I'd really prefer to not > > > reuse types from a spec that the Polaris project doesn't own. > > > > > > A (controversial) thing is how to represent fully-qualified identifiers > > > (think: table-identifer and namespace-elements) as path-parameters. > > > Iceberg uses 0x1F as the element separator, but that is a control > > > character, which can be problematic. I'd propose to use a different > > > escaping that is in 99.9% of all cases both easy/intuitive to read and > > > write and prevents control characters and ambiguous paths ([1]). > > > Reference: [2] > > > > > > Robert > > > > > > [1] Jakarta Servlet Spec 6.0 - > > > > > > > > https://jakarta.ee/specifications/servlet/6.0/jakarta-servlet-spec-6.0#uri-path-canonicalization > > > [2] > > > > > > > > https://github.com/projectnessie/nessie/blob/fbbbf8ae7ac0d961997c685afd32d5dceed90efc/api/model/src/main/java/org/projectnessie/model/Elements.java#L80-L121 > > > > > > On 25.02.25 00:48, yun zou wrote: > > > > "If Polaris were to add new endpoints under /catalog/v1 and then > > Iceberg > > > > releases a v2 REST Catalog API what happens to the Polaris-specific > > > > endpoints?" > > > > > > > > -- I think that is a great point, I am actually with Dmitri now -- a > > new > > > > prefix could be beneficial for Polaris API to evolve independently in > > the > > > > future. > > > > > > > > For the new prefix name part, I think we will definitely need to > > continue > > > > supporting the /catalog/v1 for the Iceberg V1 APIs to avoid breaking > > any > > > > client that already works with Polaris today. Maybe when Iceberg > > evolves > > > to > > > > v2, we can start moving then to /catalog/iceberg/. > > > > > > > > For our new set of APIs includes generic tables and policy, maybe even > > > > volume move on, I actually prefer /catalog/polaris, like Dmitri > > > mentioned, > > > > it indicates clearly that set of API evolves by Polaris, and we can > > embed > > > > all new object specific APIs under this prefix. > > > > > > > > Best Regards, > > > > Yun > > > > > > > > On Mon, Feb 24, 2025 at 2:50 PM Dmitri Bourlatchkov <di...@apache.org> > > > > wrote: > > > > > > > >> The current /catalog/v1 URI prefix encapsulates the Iceberg REST > > Catalog > > > >> API. This API is not owned by Polaris. > > > >> > > > >> If Polaris were to add new endpoints under /catalog/v1 and then > > Iceberg > > > >> releases a v2 REST Catalog API what happens to the Polaris-specific > > > >> endpoints? What if Iceberg adds an endpoint that matches the Polaris > > > >> endpoint URI (unlikely but possible)? > > > >> > > > >> From my POV this is the primary reason for putting the new API under > > a > > > >> different prefix. > > > >> > > > >> A secondary concern is that listing methods in the Iceberg REST API > > will > > > >> certainly not include entities created via the proposed new API, so > > > mixing > > > >> new create endpoints with old lising endpoints does not make sense to > > me > > > >> from the REST API design perspective. > > > >> > > > >> As for a practical proposal, how about the following? > > > >> > > > >> * /catalog/v1 - remains supported for Iceberg REST API v1, but > > > deprecated > > > >> and will not be used for v2. > > > >> * /catalog/iceberg/ - encapsulates the Iceberg REST API v1 (and future > > > API > > > >> versions). > > > >> * /catalog/generic/v1 - the API proposed in the referenced document. > > > >> * /management - current Polaris Management API. > > > >> > > > >> Now, "generic" is, of course, a naming concern. Other opinions are > > > welcome > > > >> regarding how to call the new API. > > > >> > > > >> Options that look reasonable to me (in order of preference): > > > >> > > > >> * /catalog/polaris/v1 - this implies that Polaris evolves this API as > > > the > > > >> primary API for all things catalogued by Polaris (not including roles, > > > >> grants, etc., which fall under /management) > > > >> * /catalog/api/v1 - same as above, but different path segment > > > >> * /catalog/storage-object/v1 > > > >> * /catalog/generic/v1 - Works fine for the current proposal, but it > > will > > > >> look awkward if we have to add typed / specialized endpoints later (so > > > >> those API extensions may need yet another prefix). > > > >> * /catalog/generic-table/v1 - same as above > > > >> * /catalog/storage-entity/v1 > > > >> > > > >> Cheers, > > > >> Dmitri. > > > >> > > > >> On Mon, Feb 24, 2025 at 3:10 PM yun zou <yunzou.colost...@gmail.com> > > > >> wrote: > > > >> > > > >>> Hi All, > > > >>> > > > >>> Thanks all for attending the review today and providing your valuable > > > >>> feedback, I really appreciate it ! And apologize for the late notice > > > for > > > >>> the review meeting here. > > > >>> > > > >>> I think we had a good discussion, and agreed to > > > >>> 1) Move on to provide a new Spark Catalog Plugin and new set of APIs > > > with > > > >>> restricted scope to table managements, but keep the doors open for > > > future > > > >>> extensions includes new fields or even new APIs. > > > >>> 2) Update the field create_at_ms to catalog_register_at to be more > > > clear > > > >>> that the timestamp is for the registration time with Polaris. > > > >>> > > > >>> We will start working on figuring out more implementation details for > > > the > > > >>> plugins as well as refactoring needed for Apache Polaris. If it turns > > > out > > > >>> too complicated, we can have a separate review or discussion about > > the > > > >>> implementation part also. > > > >>> > > > >>> As a follow up of the meeting, we would like to open this thread for > > > >>> discussion for the URI prefix for the new endpoint. As Dmitri brought > > > up > > > >> on > > > >>> the design doc, since this set of APIs are new and not restricted by > > > >>> Iceberg anymore, shall we introduce a new prefix like > > > >> /polaris/catalog/v1/ > > > >>> instead of reuse the current /catalog/v1? > > > >>> > > > >>> I think my concern about this proposal is that it seems to diverge > > how > > > >> the > > > >>> APIs path are formalized today, the APIs in Polaris today use a path > > > that > > > >>> is prefixed by functionality, like /catalog/ or /management/. If we > > > start > > > >>> using /polaris/catalog/, shall we change the /management also to > > > >>> /polaris/management ? Shall we move the new Policy Engine related > > APIs > > > to > > > >>> /polaris/catalog also? > > > >>> > > > >>> Another choice we have is to use /polaris-catalog/v1 instead > > > /catalog/v1 > > > >> as > > > >>> a prefix, which aligns with the current API path better. Then we have > > > the > > > >>> same question, what do we do with the Policy Engine APIs? The > > Policies > > > >> can > > > >>> eventually be applied to both table entities created through the > > > /catalog > > > >>> APIs and /polaris/catalog APIs. > > > >>> > > > >>> Overall, I would prefer to stay consistent with the current APIs and > > > >>> continue to use /catalog/v1, since regardless of Iceberg or Generic > > > >> tables, > > > >>> they are just entities managed by Polaris, and the APIs don't have > > > >>> 'iceberg' in the path also. I would like to hear what people think > > > about > > > >>> this? > > > >>> > > > >>> Link to the doc: > > > >>> > > > >>> > > > >> > > > > > https://docs.google.com/document/d/1_R9jBIwoH3CV9G7gSoRJPQVEcOsROp4IEiGoeYeQE8A/edit?pli=1&tab=t.0 > > > >>> Best Regards, > > > >>> Yun > > > >>> > > > -- > > > Robert Stupp > > > @snazy > > > > > > > >