You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "snazy (via GitHub)" <gi...@apache.org> on 2025/11/24 08:33:48 UTC
[PR] NoSQL: CDI / Quarkus [polaris]
snazy opened a new pull request, #3135:
URL: https://github.com/apache/polaris/pull/3135
Adds the Quarkus/CDI code handling Quarkus specifics.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2566267350
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
NoSql is a WIP feature. There is no dependency outside of Polaris. If you are talking about the current Polaris modules, can you point it out? We should fix that as well before shipping it.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2565718643
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
IMHO its NoSQL specific to folks who checks the code and sees backend is under NoSQL module, but not for user POV it can be confusing never the less we already have `polaris.persistence.relational.jdbc` in the confs where relational-jdbc is the module, IMHO we should keep it consistent.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2586340511
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
LGTM. Thanks @snazy and @dimas-b for the changes! cc @singhpk234
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2587188505
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/ObservingBackend.java:
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import io.micrometer.core.annotation.Counted;
+import io.micrometer.core.annotation.Timed;
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.config.MeterFilter;
+import io.opentelemetry.instrumentation.annotations.SpanAttribute;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
+import jakarta.annotation.Nonnull;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Default;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Singleton;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import org.apache.polaris.ids.api.IdGenerator;
+import org.apache.polaris.ids.api.MonotonicClock;
+import org.apache.polaris.persistence.nosql.api.Persistence;
+import org.apache.polaris.persistence.nosql.api.PersistenceParams;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.FetchedObj;
+import org.apache.polaris.persistence.nosql.api.backend.PersistId;
+import org.apache.polaris.persistence.nosql.api.backend.WriteObj;
+import org.apache.polaris.persistence.nosql.api.obj.ObjRef;
+import org.apache.polaris.persistence.nosql.api.ref.Reference;
+
+/** Provides telemetry and tracing for all persistence backend operations. */
+@ApplicationScoped
+@Default
+public class ObservingBackend implements Backend {
+ public static final String TELEMETRY_PREFIX = "polaris.persistence.nosql";
Review Comment:
nit / optional : may be we can include `.` in prefix `polaris.persistence.nosql.`
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2566204719
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
@flyrain : as I mentioned above there are existing downstream dependencies on these config names. This aligns with the lenient approach to keeping `polaris-core` at java 11 as [discussed](https://github.com/apache/polaris/discussions/100#discussioncomment-10267097) in #100. WDYT?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2565883860
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
@singhpk234 : I agree with your point in general, but this particular config is already in use in our downstream projects (from where we're contributing NoSQL code). Changing the config name is possible, but very inconvenient at this point (recall the very long NoSQL persistence discussion process). If you do not mind, I'd prefer to merge with the existing config names. As @snazy pointed out, config namespaces do not overlap, so there's no technical ambiguity.
End users in most cases do not need to rationalize config names, IMHO, they'd mostly follow documentation, which will instruct they what exact config names to set depending on the use case.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2585494340
##########
persistence/nosql/persistence/correctness/README.md:
##########
@@ -29,8 +29,8 @@ The `correctnessManualTest` task however is meant to be run _manually_ against a
providing the necessary backend configuration via system properties. For example:
```bash
./gradlew :polaris-persistence-nosql-correctness:correctnessManualTest \
- -Dpolaris.persistence.backend=MongoDb \
- -Dpolaris.persistence.backend.mongodb.uri=mongodb://localhost:27017/test
- -Dpolaris.persistence.backend.mongodb.databaseName=polaris_mongo_test
+ -Dpolaris.persistence.nosql=MongoDb \
Review Comment:
`polaris.persistence.nosql.backend=...`?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #3135:
URL: https://github.com/apache/polaris/pull/3135#issuecomment-3587038774
`dev` ML: https://lists.apache.org/thread/vmdpb45j1nmmlhswz7fm87gxgoldmr28
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2557183760
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/MongoDbBackendBuilder.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import com.mongodb.client.MongoClient;
+import io.quarkus.arc.Arc;
+import io.quarkus.mongodb.runtime.MongoClientBeanUtil;
+import io.quarkus.mongodb.runtime.MongoClients;
+import jakarta.enterprise.context.Dependent;
+import jakarta.inject.Inject;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.mongodb.MongoDbBackendConfig;
+import org.apache.polaris.persistence.nosql.mongodb.MongoDbBackendFactory;
+import org.eclipse.microprofile.config.inject.ConfigProperty;
+
+@BackendType(MongoDbBackendFactory.NAME)
+@Dependent
+class MongoDbBackendBuilder implements BackendBuilder {
Review Comment:
Would it be possible to (re-)structure CDI objects in a way that will not mix multiple backends in the same general-purpose module?
I imagine when new backends are added, it would be preferable to keep their code isolated in their respective modules as much as possible.
In other words, is it possible to build a server with classes from just one particular backend implementation present on the class path?
##########
LICENSE:
##########
@@ -338,6 +338,9 @@ This product includes code from Project Nessie.
* helm/polaris/templates/servicemonitor.yaml
* helm/polaris/templates/storage.yaml
+Code underneath the components/persistence directory, especially pluggable object types, index related, cache,
Review Comment:
`components/persistence` does not exist in the current `main` 🤔 Shall we list individual files (as above)?
##########
persistence/nosql/persistence/cdi/quarkus/build.gradle.kts:
##########
@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+plugins {
+ id("org.kordamp.gradle.jandex")
+ id("polaris-server")
+}
+
+description = "Polaris NoSQL persistence, providers for Quarkus."
+
+dependencies {
+ implementation(project(":polaris-persistence-nosql-cdi-common"))
+ implementation(project(":polaris-persistence-nosql-api"))
+ implementation(project(":polaris-persistence-nosql-inmemory"))
+ implementation(project(":polaris-persistence-nosql-mongodb"))
Review Comment:
side note: this is related to my comment on `MongoDbBackendBuilder`
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2560125577
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/MongoDbBackendBuilder.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import com.mongodb.client.MongoClient;
+import io.quarkus.arc.Arc;
+import io.quarkus.mongodb.runtime.MongoClientBeanUtil;
+import io.quarkus.mongodb.runtime.MongoClients;
+import jakarta.enterprise.context.Dependent;
+import jakarta.inject.Inject;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.mongodb.MongoDbBackendConfig;
+import org.apache.polaris.persistence.nosql.mongodb.MongoDbBackendFactory;
+import org.eclipse.microprofile.config.inject.ConfigProperty;
+
+@BackendType(MongoDbBackendFactory.NAME)
+@Dependent
+class MongoDbBackendBuilder implements BackendBuilder {
Review Comment:
Fair enough.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2566233533
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
NonSql is a WIP feature. I'm confused by the "existing downstream dependencies". Can you elaborate it?
And also how does it related to polaris-core and java 11?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2563674508
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/ObservingBackend.java:
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import io.micrometer.core.annotation.Counted;
+import io.micrometer.core.annotation.Timed;
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.config.MeterFilter;
+import io.opentelemetry.instrumentation.annotations.SpanAttribute;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
+import jakarta.annotation.Nonnull;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Default;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Singleton;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import org.apache.polaris.ids.api.IdGenerator;
+import org.apache.polaris.ids.api.MonotonicClock;
+import org.apache.polaris.persistence.nosql.api.Persistence;
+import org.apache.polaris.persistence.nosql.api.PersistenceParams;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.FetchedObj;
+import org.apache.polaris.persistence.nosql.api.backend.PersistId;
+import org.apache.polaris.persistence.nosql.api.backend.WriteObj;
+import org.apache.polaris.persistence.nosql.api.obj.ObjRef;
+import org.apache.polaris.persistence.nosql.api.ref.Reference;
+
+/** Provides telemetry and tracing for all persistence backend operations. */
+@ApplicationScoped
+@Default
+public class ObservingBackend implements Backend {
+ public static final String TELEMETRY_PREFIX = "polaris.persistence";
Review Comment:
Not sure a change here is necessary. The service can have only one kind of persistence.
Telemetry keys are already quite long, so I'd like to avoid making it even longer.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2565883860
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
@singhpk234 : I agree with your point in general, but this particular config is already in use in our downstream projects (from where we're contributing NoSQL code). Changing the config name is possible, but very inconvenient at this point (recall the very long NoSQL persistence discussion process). If you do not mind, I'd prefer to merge with the existing config names. As @snazy pointed out, config namespaces do not overlap, so there's no technical ambiguity.
End users in most cases do not need to rationalize config names, IMHO, they'll mostly follow documentation, which will instruct they what exact config names to set depending on the use case.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2565777921
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/ObservingBackend.java:
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import io.micrometer.core.annotation.Counted;
+import io.micrometer.core.annotation.Timed;
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.config.MeterFilter;
+import io.opentelemetry.instrumentation.annotations.SpanAttribute;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
+import jakarta.annotation.Nonnull;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Default;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Singleton;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import org.apache.polaris.ids.api.IdGenerator;
+import org.apache.polaris.ids.api.MonotonicClock;
+import org.apache.polaris.persistence.nosql.api.Persistence;
+import org.apache.polaris.persistence.nosql.api.PersistenceParams;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.FetchedObj;
+import org.apache.polaris.persistence.nosql.api.backend.PersistId;
+import org.apache.polaris.persistence.nosql.api.backend.WriteObj;
+import org.apache.polaris.persistence.nosql.api.obj.ObjRef;
+import org.apache.polaris.persistence.nosql.api.ref.Reference;
+
+/** Provides telemetry and tracing for all persistence backend operations. */
+@ApplicationScoped
+@Default
+public class ObservingBackend implements Backend {
+ public static final String TELEMETRY_PREFIX = "polaris.persistence";
Review Comment:
> The service can have only one kind of persistence
JDBC will not be support this telemetry having something like polaris.persistence and not adding nosql in the confs might confuse the user that this is available for jdbc too, I resonate with the keys becoming a little bit log on adding nosql, but from user experience POV it will helpful
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2566255237
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
`polaris-core` has downstream dependencies the rely on it being compiled targeting java 11. There is no other reason against upgrading the java language level in that module.
`polaris.persistence.backend.type` has downstream dependencies that rely on that config name.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2565883860
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
@singhpk234 : I agree with your point in general, but this particular config is already in use in our downstream projects (from where we're contributing NoSQL code). Changing the config name is possible, but very inconvenient at this point (recall the very long NoSQL persistence discussion process). If you do not mind, I'd prefer to merge with the existing config names. As @snazy pointed out, config namespaces do not overlap, so there's no technical ambiguity.
End users in most cases do not need to rationalize config names, IMHO, they'll mostly follow documentation, which will instruct them what exact config names to set depending on the use case.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2567637341
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/ObservingBackend.java:
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import io.micrometer.core.annotation.Counted;
+import io.micrometer.core.annotation.Timed;
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.config.MeterFilter;
+import io.opentelemetry.instrumentation.annotations.SpanAttribute;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
+import jakarta.annotation.Nonnull;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Default;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Singleton;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import org.apache.polaris.ids.api.IdGenerator;
+import org.apache.polaris.ids.api.MonotonicClock;
+import org.apache.polaris.persistence.nosql.api.Persistence;
+import org.apache.polaris.persistence.nosql.api.PersistenceParams;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.FetchedObj;
+import org.apache.polaris.persistence.nosql.api.backend.PersistId;
+import org.apache.polaris.persistence.nosql.api.backend.WriteObj;
+import org.apache.polaris.persistence.nosql.api.obj.ObjRef;
+import org.apache.polaris.persistence.nosql.api.ref.Reference;
+
+/** Provides telemetry and tracing for all persistence backend operations. */
+@ApplicationScoped
+@Default
+public class ObservingBackend implements Backend {
+ public static final String TELEMETRY_PREFIX = "polaris.persistence";
Review Comment:
Updated the prefix to `polaris.persistence.nosql`
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2557293936
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/MongoDbBackendBuilder.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import com.mongodb.client.MongoClient;
+import io.quarkus.arc.Arc;
+import io.quarkus.mongodb.runtime.MongoClientBeanUtil;
+import io.quarkus.mongodb.runtime.MongoClients;
+import jakarta.enterprise.context.Dependent;
+import jakarta.inject.Inject;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.mongodb.MongoDbBackendConfig;
+import org.apache.polaris.persistence.nosql.mongodb.MongoDbBackendFactory;
+import org.eclipse.microprofile.config.inject.ConfigProperty;
+
+@BackendType(MongoDbBackendFactory.NAME)
+@Dependent
+class MongoDbBackendBuilder implements BackendBuilder {
Review Comment:
Sure, we can add quarkus-specific modules next to every backend. But that's way more work at this point.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy merged PR #3135:
URL: https://github.com/apache/polaris/pull/3135
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2563662887
##########
LICENSE:
##########
@@ -338,6 +338,9 @@ This product includes code from Project Nessie.
* helm/polaris/templates/servicemonitor.yaml
* helm/polaris/templates/storage.yaml
+Code underneath the components/persistence directory, especially pluggable object types, index related, cache,
Review Comment:
Hard to name "individual files", as it's only inspired. But I see this causes confusion. Removing this from the PR.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2563681571
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
`backend` is already NoSql specific. I'm not sure whether making something that's already specific is worth changing it?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2565834537
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/ObservingBackend.java:
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import io.micrometer.core.annotation.Counted;
+import io.micrometer.core.annotation.Timed;
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.config.MeterFilter;
+import io.opentelemetry.instrumentation.annotations.SpanAttribute;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
+import jakarta.annotation.Nonnull;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Default;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Singleton;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import org.apache.polaris.ids.api.IdGenerator;
+import org.apache.polaris.ids.api.MonotonicClock;
+import org.apache.polaris.persistence.nosql.api.Persistence;
+import org.apache.polaris.persistence.nosql.api.PersistenceParams;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.FetchedObj;
+import org.apache.polaris.persistence.nosql.api.backend.PersistId;
+import org.apache.polaris.persistence.nosql.api.backend.WriteObj;
+import org.apache.polaris.persistence.nosql.api.obj.ObjRef;
+import org.apache.polaris.persistence.nosql.api.ref.Reference;
+
+/** Provides telemetry and tracing for all persistence backend operations. */
+@ApplicationScoped
+@Default
+public class ObservingBackend implements Backend {
+ public static final String TELEMETRY_PREFIX = "polaris.persistence";
Review Comment:
In terms of actual prefix, indeed a given server can have only one kind of persistence. However, nothing guarantees that a given telemetry collector processes data only from servers running the same type of persistence.
Since the prefix is part of span names, having something like `polaris.persistence.nosql` here and `polaris.persistence.jdbc` in JDBC (later, hypothetically) would make sense to me.
It is not likely that span queries or alerts depend on the exact span name prefix, so appending a word to the prefix is probably a relatively safe change from the user's perspective.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2566255237
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
`polaris-core` has downstream dependencies the rely on it being compiled targeting java 11. There is no other reason to upgrading the java language level in that module.
`polaris.persistence.backend.type` has downstream dependencies that rely on that config name.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2585502326
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
@flyrain : please see the latest config name change. I believe they align with your suggestion and this comment thread is not a blocker for merging. WDYT?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2565807668
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/ObservingBackend.java:
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import io.micrometer.core.annotation.Counted;
+import io.micrometer.core.annotation.Timed;
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.config.MeterFilter;
+import io.opentelemetry.instrumentation.annotations.SpanAttribute;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
+import jakarta.annotation.Nonnull;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Default;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Singleton;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import org.apache.polaris.ids.api.IdGenerator;
+import org.apache.polaris.ids.api.MonotonicClock;
+import org.apache.polaris.persistence.nosql.api.Persistence;
+import org.apache.polaris.persistence.nosql.api.PersistenceParams;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.FetchedObj;
+import org.apache.polaris.persistence.nosql.api.backend.PersistId;
+import org.apache.polaris.persistence.nosql.api.backend.WriteObj;
+import org.apache.polaris.persistence.nosql.api.obj.ObjRef;
+import org.apache.polaris.persistence.nosql.api.ref.Reference;
+
+/** Provides telemetry and tracing for all persistence backend operations. */
+@ApplicationScoped
+@Default
+public class ObservingBackend implements Backend {
+ public static final String TELEMETRY_PREFIX = "polaris.persistence";
Review Comment:
From my POV it would be beneficial for the JDBC Persistence to provide a similar "observing" front end for producing OTel spans.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2566090158
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
Given the feature is still WIP, I think we should address the naming inconsistency before shipping it.
For JDBC we follow a clear prefix (`polaris.persistence.relational.jdbc`), but the NoSQL config currently doesn’t follow the same pattern.
To keep the overall configuration model consistent and intuitive for users, I believe we should align the NoSQL prefix as well before the feature stabilizes.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2560363643
##########
LICENSE:
##########
@@ -338,6 +338,9 @@ This product includes code from Project Nessie.
* helm/polaris/templates/servicemonitor.yaml
* helm/polaris/templates/storage.yaml
+Code underneath the components/persistence directory, especially pluggable object types, index related, cache,
Review Comment:
@jbonofre : WDYT?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2566258306
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
@collado-mike : wondering about your opinion on this matter, if you do not mind.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2558520605
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/ObservingBackend.java:
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import io.micrometer.core.annotation.Counted;
+import io.micrometer.core.annotation.Timed;
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.config.MeterFilter;
+import io.opentelemetry.instrumentation.annotations.SpanAttribute;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
+import jakarta.annotation.Nonnull;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Default;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Singleton;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import org.apache.polaris.ids.api.IdGenerator;
+import org.apache.polaris.ids.api.MonotonicClock;
+import org.apache.polaris.persistence.nosql.api.Persistence;
+import org.apache.polaris.persistence.nosql.api.PersistenceParams;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.FetchedObj;
+import org.apache.polaris.persistence.nosql.api.backend.PersistId;
+import org.apache.polaris.persistence.nosql.api.backend.WriteObj;
+import org.apache.polaris.persistence.nosql.api.obj.ObjRef;
+import org.apache.polaris.persistence.nosql.api.ref.Reference;
+
+/** Provides telemetry and tracing for all persistence backend operations. */
+@ApplicationScoped
+@Default
+public class ObservingBackend implements Backend {
+ public static final String TELEMETRY_PREFIX = "polaris.persistence";
Review Comment:
same comment as above https://github.com/apache/polaris/pull/3135/files#r2558515915
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
shouldn't the backend configuration have nosql in the prefix like we do in relational-jdbc
for example :
polaris.persistence.relational.jdbc
https://polaris.apache.org/in-dev/unreleased/configuration/
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2565777921
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/ObservingBackend.java:
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import io.micrometer.core.annotation.Counted;
+import io.micrometer.core.annotation.Timed;
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.config.MeterFilter;
+import io.opentelemetry.instrumentation.annotations.SpanAttribute;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
+import jakarta.annotation.Nonnull;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Default;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Singleton;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import org.apache.polaris.ids.api.IdGenerator;
+import org.apache.polaris.ids.api.MonotonicClock;
+import org.apache.polaris.persistence.nosql.api.Persistence;
+import org.apache.polaris.persistence.nosql.api.PersistenceParams;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.FetchedObj;
+import org.apache.polaris.persistence.nosql.api.backend.PersistId;
+import org.apache.polaris.persistence.nosql.api.backend.WriteObj;
+import org.apache.polaris.persistence.nosql.api.obj.ObjRef;
+import org.apache.polaris.persistence.nosql.api.ref.Reference;
+
+/** Provides telemetry and tracing for all persistence backend operations. */
+@ApplicationScoped
+@Default
+public class ObservingBackend implements Backend {
+ public static final String TELEMETRY_PREFIX = "polaris.persistence";
Review Comment:
> The service can have only one kind of persistence
JDBC will not be able to support this telemetry at the moment, having something like polaris.persistence and not adding nosql in the confs might confuse the user that this is available for jdbc too, I resonate with the keys becoming a little bit log on adding nosql, but from user experience POV it will helpful
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "singhpk234 (via GitHub)" <gi...@apache.org>.
singhpk234 commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2565830947
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/ObservingBackend.java:
##########
@@ -0,0 +1,263 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import io.micrometer.core.annotation.Counted;
+import io.micrometer.core.annotation.Timed;
+import io.micrometer.core.instrument.Meter;
+import io.micrometer.core.instrument.Tag;
+import io.micrometer.core.instrument.config.MeterFilter;
+import io.opentelemetry.instrumentation.annotations.SpanAttribute;
+import io.opentelemetry.instrumentation.annotations.WithSpan;
+import jakarta.annotation.Nonnull;
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Default;
+import jakarta.enterprise.inject.Produces;
+import jakarta.inject.Singleton;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Function;
+import org.apache.polaris.ids.api.IdGenerator;
+import org.apache.polaris.ids.api.MonotonicClock;
+import org.apache.polaris.persistence.nosql.api.Persistence;
+import org.apache.polaris.persistence.nosql.api.PersistenceParams;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.FetchedObj;
+import org.apache.polaris.persistence.nosql.api.backend.PersistId;
+import org.apache.polaris.persistence.nosql.api.backend.WriteObj;
+import org.apache.polaris.persistence.nosql.api.obj.ObjRef;
+import org.apache.polaris.persistence.nosql.api.ref.Reference;
+
+/** Provides telemetry and tracing for all persistence backend operations. */
+@ApplicationScoped
+@Default
+public class ObservingBackend implements Backend {
+ public static final String TELEMETRY_PREFIX = "polaris.persistence";
Review Comment:
Agree it would be something that would be really helpful for the users ! though even in the scenario we would want jdbc and NoSQL will have their own prefix ?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2566204719
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
@flyrain : as I mentioned above there are existing downstream dependencies on these config names. This aligns with the lenient approach to keeping `polaris-core` at java 11 as [discussed](https://github.com/apache/polaris/discussions/100#discussioncomment-10267097) in #100. WDYT?
That said, it is certainly possible to refactor the config names later, while adding backward compatibility, but for the initial NoSQL persistence contribution that would add undue hardship, IMHO.
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #3135:
URL: https://github.com/apache/polaris/pull/3135#discussion_r2566296766
##########
persistence/nosql/persistence/cdi/quarkus/src/main/java/org/apache/polaris/persistence/nosql/quarkus/backend/BackendProvider.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.polaris.persistence.nosql.quarkus.backend;
+
+import static java.lang.String.format;
+
+import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.enterprise.inject.Any;
+import jakarta.enterprise.inject.Instance;
+import jakarta.enterprise.inject.Produces;
+import java.util.Optional;
+import org.apache.polaris.persistence.nosql.api.backend.Backend;
+import org.apache.polaris.persistence.nosql.api.backend.BackendConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@ApplicationScoped
+class BackendProvider {
+ private static final Logger LOGGER = LoggerFactory.getLogger(BackendProvider.class);
+
+ @Produces
+ @ApplicationScoped
+ @NotObserved
+ Backend backend(
+ BackendConfiguration backendConfiguration, @Any Instance<BackendBuilder> backendBuilders) {
+
+ var backendName =
+ backendConfiguration
+ .type()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ "Mandatory configuration option polaris.persistence.backend.type is missing!"));
Review Comment:
@flyrain :
> There is no dependency outside of Polaris
How do you know that?
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] NoSQL: CDI / Quarkus [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on PR #3135:
URL: https://github.com/apache/polaris/pull/3135#issuecomment-3586992076
> let's move that discussion to the `dev` ML.
+1
--
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: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org