roryqi commented on PR #10536: URL: https://github.com/apache/gravitino/pull/10536#issuecomment-4132697986
> Makes sense. I agree that HTTP-layer behavior should not be pushed all the way down to the lower layer. > > My main concern with the current approach is not `getTableMetadataLocation(...)` itself, but that the fast path returns before going through the normal `loadTable` chain, which means it may bypass existing event/audit hooks. > > If we want to encapsulate this better, I think we probably need a better `loadTable` result model first, for example a `ConditionalLoadResult` that can represent either `not modified` or `loaded`. That would let us keep the conditional-load logic on the `loadTable` path while still preserving the existing event/hook semantics. > > @jerryshao @roryqi WDYT? It's ok that we don't call the event or hook, because we don't call the method `loadTable`. If requests failed because of request parameters, we won't ttigger the event and hook. I feel this is a check, too. -- 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]
