obelix74 commented on PR #4115:
URL: https://github.com/apache/polaris/pull/4115#issuecomment-4784248938

   > Thanks Anand! this is much closer to the split we discussed. The 
JDBC-specific pieces are no longer in generic `persistence/relational-jdbc`, 
the default implementations are under `extensions`, and the query path now 
returns 501 instead of an empty successful response.
   > 
   > I still think this PR should stop at the reporting path:
   > 
   > * put the stable reporting SPI in `polaris-core`;
   > * keep the default impl in `extensions`, preferably no-op by default and 
log as opt-in;
   > * keep the REST query service/spec/docs out of this PR and move them with 
the durable query implementation;
   > * move/defer the remaining core metrics persistence records/manager to the 
durable extension work.
   > 
   > That keeps this PR small and clear: reporting SPI + default 
implementation. Query, pagination, filters, storage records, and durable 
persistence can land together in the follow-up PR.
   
   I've rescoped this PR to just the reporting SPI + default implementation.
   
   What's in this PR now:
   
   - IcebergMetricsReporter SPI moved to polaris-core 
(org.apache.polaris.core.metrics) — the stable reporting interface, 
framework-agnostic
   - LoggingMetricsReporter in extensions/metrics-reports — opt-in via 
polaris.iceberg-metrics.reporting.type=log
   - NoOpMetricsReporter in extensions/metrics-reports — new default (changed 
from log)
   - MetricsReportingConfiguration in runtime/service — CDI wiring with 
graceful fallback if no reporter is on the classpath
   
   Deferred to PR #4756 (implement_metrics_jdbc_extension):
   
   - REST query service, OpenAPI spec, and API docs
   - TABLE_READ_METRICS / LIST_TABLE_METRICS auth additions
   - Durable JDBC extension wiring (PersistingMetricsReporter, 
JdbcMetricsPersistence)
   
   The diff is now: 3 files renamed/moved (PolarisMetricsReporter → 
IcebergMetricsReporter in core, DefaultMetricsReporter → LoggingMetricsReporter 
in extensions), 1 new file (NoOpMetricsReporter), and wiring updates in 
IcebergCatalogHandler / ServiceProducers.


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