Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
twalthr merged PR #27394: URL: https://github.com/apache/flink/pull/27394 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
lihaosky commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2766694362
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/TableEnvironmentImpl.java:
##
@@ -263,6 +265,19 @@ public static TableEnvironmentImpl
create(EnvironmentSettings settings) {
? settings.getCatalogStore()
: catalogStoreFactory.createCatalogStore();
+final SecretStoreFactory secretStoreFactory =
+TableFactoryUtil.findAndCreateSecretStoreFactory(
+settings.getConfiguration(), userClassLoader);
+final SecretStoreFactory.Context secretStoreContext =
+TableFactoryUtil.buildSecretStoreFactoryContext(
+settings.getConfiguration(), userClassLoader);
+secretStoreFactory.open(secretStoreContext);
+// TODO (FLINK-38261): pass secret store to catalog manager for
encryption/decryption
+final SecretStore secretStore =
+settings.getSecretStore() != null
+? settings.getSecretStore()
Review Comment:
I created a new PR for the refactoring:
https://github.com/apache/flink/pull/27531
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/EnvironmentSettings.java:
##
@@ -154,6 +158,12 @@ public Optional getSqlFactory() {
return Optional.ofNullable(sqlFactory);
}
+@Internal
+@Nullable
+public SecretStore getSecretStore() {
Review Comment:
Put in new PR: https://github.com/apache/flink/pull/27531
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
twalthr commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2759849110
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/secret/GenericInMemorySecretStore.java:
##
@@ -0,0 +1,114 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.table.secret.exceptions.SecretException;
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.core.JsonProcessingException;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.core.type.TypeReference;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.ObjectMapper;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import static org.apache.flink.util.Preconditions.checkNotNull;
+
+/**
+ * A generic in-memory implementation of both {@link ReadableSecretStore} and
{@link
+ * WritableSecretStore}.
+ *
+ * This implementation stores secrets in memory as plaintext JSON strings.
It is suitable for
+ * testing and development purposes but should not be used in production
environments as secrets are
+ * not encrypted.
+ */
+@Internal
+public class GenericInMemorySecretStore implements ReadableSecretStore,
WritableSecretStore {
+
+private static final ObjectMapper objectMapper = new ObjectMapper();
Review Comment:
Store an immutable `Map`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
twalthr commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2759785912
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/TableEnvironmentImpl.java:
##
@@ -263,6 +265,19 @@ public static TableEnvironmentImpl
create(EnvironmentSettings settings) {
? settings.getCatalogStore()
: catalogStoreFactory.createCatalogStore();
+final SecretStoreFactory secretStoreFactory =
+TableFactoryUtil.findAndCreateSecretStoreFactory(
+settings.getConfiguration(), userClassLoader);
+final SecretStoreFactory.Context secretStoreContext =
+TableFactoryUtil.buildSecretStoreFactoryContext(
+settings.getConfiguration(), userClassLoader);
+secretStoreFactory.open(secretStoreContext);
+// TODO (FLINK-38261): pass secret store to catalog manager for
encryption/decryption
+final SecretStore secretStore =
+settings.getSecretStore() != null
+? settings.getSecretStore()
Review Comment:
We should not discover a factory if a secret store is already provided.
Please also update the CatalogStoreFactory above which has the same issue. Also
the `TableFactoryUtil` class is kind of deprecated. Move the methods for both
secret and catalog store to a new
`org.apache.flink.table.api.internal.ApiFactoryUtil`
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/EnvironmentSettings.java:
##
@@ -154,6 +158,12 @@ public Optional getSqlFactory() {
return Optional.ofNullable(sqlFactory);
}
+@Internal
+@Nullable
+public SecretStore getSecretStore() {
Review Comment:
Use Optional, maybe also for CatalogStore for consistency.
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/secret/GenericInMemorySecretStore.java:
##
@@ -0,0 +1,114 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.table.secret.exceptions.SecretException;
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.core.JsonProcessingException;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.core.type.TypeReference;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.ObjectMapper;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import static org.apache.flink.util.Preconditions.checkNotNull;
+
+/**
+ * A generic in-memory implementation of both {@link ReadableSecretStore} and
{@link
+ * WritableSecretStore}.
+ *
+ * This implementation stores secrets in memory as plaintext JSON strings.
It is suitable for
+ * testing and development purposes but should not be used in production
environments as secrets are
+ * not encrypted.
+ */
+@Internal
+public class GenericInMemorySecretStore implements ReadableSecretStore,
WritableSecretStore {
+
+private static final ObjectMapper objectMapper = new ObjectMapper();
Review Comment:
No need for Jackson deps here. We can store Java objects and avoid ser/de.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
lihaosky commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2742826731
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/TableEnvironmentImpl.java:
##
@@ -263,6 +265,19 @@ public static TableEnvironmentImpl
create(EnvironmentSettings settings) {
? settings.getCatalogStore()
: catalogStoreFactory.createCatalogStore();
+final SecretStoreFactory secretStoreFactory =
+TableFactoryUtil.findAndCreateSecretStoreFactory(
+settings.getConfiguration(), userClassLoader);
+final SecretStoreFactory.Context secretStoreContext =
+TableFactoryUtil.buildSecretStoreFactoryContext(
+settings.getConfiguration(), userClassLoader);
+secretStoreFactory.open(secretStoreContext);
+// TODO: pass secret store to catalog manager for encryption/decryption
Review Comment:
Added jira
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/factories/TableFactoryUtilTest.java:
##
@@ -0,0 +1,222 @@
+/*
+ * 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.flink.table.factories;
+
+import org.apache.flink.configuration.Configuration;
+import org.apache.flink.table.catalog.CommonCatalogOptions;
+import org.apache.flink.table.catalog.FileCatalogStoreFactory;
+import org.apache.flink.table.catalog.GenericInMemoryCatalogStoreFactory;
+import org.apache.flink.table.secret.CommonSecretOptions;
+import org.apache.flink.table.secret.GenericInMemorySecretStoreFactory;
+import org.apache.flink.table.secret.SecretStoreFactory;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.File;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/** Tests for {@link TableFactoryUtil}. */
+class TableFactoryUtilTest {
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithGenericInMemory() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"generic_in_memory");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithFile() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+assertThat(factory).isInstanceOf(FileCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithDefaultKind() {
+Configuration configuration = new Configuration();
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
Review Comment:
Parameterized them
##
flink-table/flink-table-common/src/main/java/org/apache/flink/table/secret/exceptions/SecretException.java:
##
@@ -0,0 +1,61 @@
+/*
+ * 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,
+
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738487943
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreFactoryTest.java:
##
@@ -0,0 +1,64 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.factories.FactoryUtil;
+import org.apache.flink.table.factories.FactoryUtil.DefaultSecretStoreContext;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+
+/** Test for {@link GenericInMemorySecretStoreFactory}. */
+public class GenericInMemorySecretStoreFactoryTest {
+
+@Test
+void testSecretStoreInit() {
+String factoryIdentifier =
GenericInMemorySecretStoreFactory.IDENTIFIER;
+Map options = new HashMap<>();
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+final DefaultSecretStoreContext discoveryContext =
+new DefaultSecretStoreContext(options, null, classLoader);
+final SecretStoreFactory factory =
+FactoryUtil.discoverFactory(
+classLoader, SecretStoreFactory.class,
factoryIdentifier);
+factory.open(discoveryContext);
+
+SecretStore secretStore = factory.createSecretStore();
+assertThat(secretStore instanceof GenericInMemorySecretStore).isTrue();
+
+factory.close();
+}
+
+@Test
+void testFactoryIdentifier() {
+GenericInMemorySecretStoreFactory factory = new
GenericInMemorySecretStoreFactory();
+assertThat(factory.factoryIdentifier()).isEqualTo("generic_in_memory");
+}
+
+@Test
+void testRequiredAndOptionalOptions() {
+GenericInMemorySecretStoreFactory factory = new
GenericInMemorySecretStoreFactory();
+assertThat(factory.requiredOptions().isEmpty()).isTrue();
+assertThat(factory.optionalOptions().isEmpty()).isTrue();
Review Comment:
```suggestion
assertThat(factory.requiredOptions()).isEmpty();
assertThat(factory.optionalOptions()).isEmpty();
```
will not it work like this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738523132
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+
+/** Test for {@link GenericInMemorySecretStore}. */
+public class GenericInMemorySecretStoreTest {
+
+private GenericInMemorySecretStore secretStore;
+
+@BeforeEach
+void setUp() {
+secretStore = new GenericInMemorySecretStore();
+}
+
+@Test
+void testStoreAndGetSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("username", "testuser");
+secretData.put("password", "testpass");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretId).isNotNull();
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret).isNotNull();
+assertThat(retrievedSecret.get("username")).isEqualTo("testuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("testpass");
+}
+
+@Test
+void testGetNonExistentSecret() {
+assertThatThrownBy(() -> secretStore.getSecret("non-existent-id"))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID 'non-existent-id' not
found");
+}
+
+@Test
+void testGetSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.getSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testStoreSecretWithNullData() {
+assertThatThrownBy(() -> secretStore.storeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret data cannot be null");
+}
+
+@Test
+void testRemoveSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("key", "value");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretStore.getSecret(secretId)).isNotNull();
+
+secretStore.removeSecret(secretId);
+assertThatThrownBy(() -> secretStore.getSecret(secretId))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID '" + secretId + "' not
found");
+}
+
+@Test
+void testRemoveSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.removeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testRemoveNonExistentSecret() {
+// Should not throw exception, just silently remove nothing
+secretStore.removeSecret("non-existent-id");
+}
+
+@Test
+void testUpdateSecret() throws SecretNotFoundException {
+Map originalData = new HashMap<>();
+originalData.put("username", "olduser");
+originalData.put("password", "oldpass");
+
+String secretId = secretStore.storeSecret(originalData);
+
+Map updatedData = new HashMap<>();
+updatedData.put("username", "newuser");
+updatedData.put("password", "newpass");
Review Comment:
```suggestion
Map updatedData = Map.of("username", "newuser",
"password", "newpass");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructu
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738523883
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+
+/** Test for {@link GenericInMemorySecretStore}. */
+public class GenericInMemorySecretStoreTest {
+
+private GenericInMemorySecretStore secretStore;
+
+@BeforeEach
+void setUp() {
+secretStore = new GenericInMemorySecretStore();
+}
+
+@Test
+void testStoreAndGetSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("username", "testuser");
+secretData.put("password", "testpass");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretId).isNotNull();
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret).isNotNull();
+assertThat(retrievedSecret.get("username")).isEqualTo("testuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("testpass");
+}
+
+@Test
+void testGetNonExistentSecret() {
+assertThatThrownBy(() -> secretStore.getSecret("non-existent-id"))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID 'non-existent-id' not
found");
+}
+
+@Test
+void testGetSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.getSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testStoreSecretWithNullData() {
+assertThatThrownBy(() -> secretStore.storeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret data cannot be null");
+}
+
+@Test
+void testRemoveSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("key", "value");
Review Comment:
```suggestion
Map secretData = Map.of("key", "value");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738522329
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+
+/** Test for {@link GenericInMemorySecretStore}. */
+public class GenericInMemorySecretStoreTest {
+
+private GenericInMemorySecretStore secretStore;
+
+@BeforeEach
+void setUp() {
+secretStore = new GenericInMemorySecretStore();
+}
+
+@Test
+void testStoreAndGetSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("username", "testuser");
+secretData.put("password", "testpass");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretId).isNotNull();
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret).isNotNull();
+assertThat(retrievedSecret.get("username")).isEqualTo("testuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("testpass");
+}
+
+@Test
+void testGetNonExistentSecret() {
+assertThatThrownBy(() -> secretStore.getSecret("non-existent-id"))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID 'non-existent-id' not
found");
+}
+
+@Test
+void testGetSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.getSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testStoreSecretWithNullData() {
+assertThatThrownBy(() -> secretStore.storeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret data cannot be null");
+}
+
+@Test
+void testRemoveSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("key", "value");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretStore.getSecret(secretId)).isNotNull();
+
+secretStore.removeSecret(secretId);
+assertThatThrownBy(() -> secretStore.getSecret(secretId))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID '" + secretId + "' not
found");
+}
+
+@Test
+void testRemoveSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.removeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testRemoveNonExistentSecret() {
+// Should not throw exception, just silently remove nothing
+secretStore.removeSecret("non-existent-id");
+}
+
+@Test
+void testUpdateSecret() throws SecretNotFoundException {
+Map originalData = new HashMap<>();
+originalData.put("username", "olduser");
+originalData.put("password", "oldpass");
Review Comment:
```suggestion
Map originalData = Map.of("username", "olduser",
"password", "oldpass");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738520693
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+
+/** Test for {@link GenericInMemorySecretStore}. */
+public class GenericInMemorySecretStoreTest {
+
+private GenericInMemorySecretStore secretStore;
+
+@BeforeEach
+void setUp() {
+secretStore = new GenericInMemorySecretStore();
+}
+
+@Test
+void testStoreAndGetSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("username", "testuser");
+secretData.put("password", "testpass");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretId).isNotNull();
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret).isNotNull();
+assertThat(retrievedSecret.get("username")).isEqualTo("testuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("testpass");
+}
+
+@Test
+void testGetNonExistentSecret() {
+assertThatThrownBy(() -> secretStore.getSecret("non-existent-id"))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID 'non-existent-id' not
found");
+}
+
+@Test
+void testGetSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.getSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testStoreSecretWithNullData() {
+assertThatThrownBy(() -> secretStore.storeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret data cannot be null");
+}
+
+@Test
+void testRemoveSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("key", "value");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretStore.getSecret(secretId)).isNotNull();
+
+secretStore.removeSecret(secretId);
+assertThatThrownBy(() -> secretStore.getSecret(secretId))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID '" + secretId + "' not
found");
+}
+
+@Test
+void testRemoveSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.removeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testRemoveNonExistentSecret() {
+// Should not throw exception, just silently remove nothing
+secretStore.removeSecret("non-existent-id");
+}
+
+@Test
+void testUpdateSecret() throws SecretNotFoundException {
+Map originalData = new HashMap<>();
+originalData.put("username", "olduser");
+originalData.put("password", "oldpass");
+
+String secretId = secretStore.storeSecret(originalData);
+
+Map updatedData = new HashMap<>();
+updatedData.put("username", "newuser");
+updatedData.put("password", "newpass");
+
+secretStore.updateSecret(secretId, updatedData);
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret.get("username")).isEqualTo("newuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("newpass");
+}
+
+@Test
+void testUpdateNonExistentSecret() {
+Map secretData = new HashMap<>();
+secretData.put("key", "va
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738520119
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+
+/** Test for {@link GenericInMemorySecretStore}. */
+public class GenericInMemorySecretStoreTest {
+
+private GenericInMemorySecretStore secretStore;
+
+@BeforeEach
+void setUp() {
+secretStore = new GenericInMemorySecretStore();
+}
+
+@Test
+void testStoreAndGetSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("username", "testuser");
+secretData.put("password", "testpass");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretId).isNotNull();
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret).isNotNull();
+assertThat(retrievedSecret.get("username")).isEqualTo("testuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("testpass");
+}
+
+@Test
+void testGetNonExistentSecret() {
+assertThatThrownBy(() -> secretStore.getSecret("non-existent-id"))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID 'non-existent-id' not
found");
+}
+
+@Test
+void testGetSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.getSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testStoreSecretWithNullData() {
+assertThatThrownBy(() -> secretStore.storeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret data cannot be null");
+}
+
+@Test
+void testRemoveSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("key", "value");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretStore.getSecret(secretId)).isNotNull();
+
+secretStore.removeSecret(secretId);
+assertThatThrownBy(() -> secretStore.getSecret(secretId))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID '" + secretId + "' not
found");
+}
+
+@Test
+void testRemoveSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.removeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testRemoveNonExistentSecret() {
+// Should not throw exception, just silently remove nothing
+secretStore.removeSecret("non-existent-id");
+}
+
+@Test
+void testUpdateSecret() throws SecretNotFoundException {
+Map originalData = new HashMap<>();
+originalData.put("username", "olduser");
+originalData.put("password", "oldpass");
+
+String secretId = secretStore.storeSecret(originalData);
+
+Map updatedData = new HashMap<>();
+updatedData.put("username", "newuser");
+updatedData.put("password", "newpass");
+
+secretStore.updateSecret(secretId, updatedData);
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret.get("username")).isEqualTo("newuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("newpass");
+}
+
+@Test
+void testUpdateNonExistentSecret() {
+Map secretData = new HashMap<>();
+secretData.put("key", "va
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738519176
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+
+/** Test for {@link GenericInMemorySecretStore}. */
+public class GenericInMemorySecretStoreTest {
+
+private GenericInMemorySecretStore secretStore;
+
+@BeforeEach
+void setUp() {
+secretStore = new GenericInMemorySecretStore();
+}
+
+@Test
+void testStoreAndGetSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("username", "testuser");
+secretData.put("password", "testpass");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretId).isNotNull();
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret).isNotNull();
+assertThat(retrievedSecret.get("username")).isEqualTo("testuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("testpass");
+}
+
+@Test
+void testGetNonExistentSecret() {
+assertThatThrownBy(() -> secretStore.getSecret("non-existent-id"))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID 'non-existent-id' not
found");
+}
+
+@Test
+void testGetSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.getSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testStoreSecretWithNullData() {
+assertThatThrownBy(() -> secretStore.storeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret data cannot be null");
+}
+
+@Test
+void testRemoveSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("key", "value");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretStore.getSecret(secretId)).isNotNull();
+
+secretStore.removeSecret(secretId);
+assertThatThrownBy(() -> secretStore.getSecret(secretId))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID '" + secretId + "' not
found");
+}
+
+@Test
+void testRemoveSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.removeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testRemoveNonExistentSecret() {
+// Should not throw exception, just silently remove nothing
+secretStore.removeSecret("non-existent-id");
+}
+
+@Test
+void testUpdateSecret() throws SecretNotFoundException {
+Map originalData = new HashMap<>();
+originalData.put("username", "olduser");
+originalData.put("password", "oldpass");
+
+String secretId = secretStore.storeSecret(originalData);
+
+Map updatedData = new HashMap<>();
+updatedData.put("username", "newuser");
+updatedData.put("password", "newpass");
+
+secretStore.updateSecret(secretId, updatedData);
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret.get("username")).isEqualTo("newuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("newpass");
+}
+
+@Test
+void testUpdateNonExistentSecret() {
+Map secretData = new HashMap<>();
+secretData.put("key", "va
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738518587
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+
+/** Test for {@link GenericInMemorySecretStore}. */
+public class GenericInMemorySecretStoreTest {
+
+private GenericInMemorySecretStore secretStore;
+
+@BeforeEach
+void setUp() {
+secretStore = new GenericInMemorySecretStore();
+}
+
+@Test
+void testStoreAndGetSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("username", "testuser");
+secretData.put("password", "testpass");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretId).isNotNull();
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret).isNotNull();
+assertThat(retrievedSecret.get("username")).isEqualTo("testuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("testpass");
+}
+
+@Test
+void testGetNonExistentSecret() {
+assertThatThrownBy(() -> secretStore.getSecret("non-existent-id"))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID 'non-existent-id' not
found");
+}
+
+@Test
+void testGetSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.getSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testStoreSecretWithNullData() {
+assertThatThrownBy(() -> secretStore.storeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret data cannot be null");
+}
+
+@Test
+void testRemoveSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("key", "value");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretStore.getSecret(secretId)).isNotNull();
+
+secretStore.removeSecret(secretId);
+assertThatThrownBy(() -> secretStore.getSecret(secretId))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID '" + secretId + "' not
found");
+}
+
+@Test
+void testRemoveSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.removeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testRemoveNonExistentSecret() {
+// Should not throw exception, just silently remove nothing
+secretStore.removeSecret("non-existent-id");
+}
+
+@Test
+void testUpdateSecret() throws SecretNotFoundException {
+Map originalData = new HashMap<>();
+originalData.put("username", "olduser");
+originalData.put("password", "oldpass");
+
+String secretId = secretStore.storeSecret(originalData);
+
+Map updatedData = new HashMap<>();
+updatedData.put("username", "newuser");
+updatedData.put("password", "newpass");
+
+secretStore.updateSecret(secretId, updatedData);
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret.get("username")).isEqualTo("newuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("newpass");
+}
+
+@Test
+void testUpdateNonExistentSecret() {
+Map secretData = new HashMap<>();
+secretData.put("key", "va
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738517855
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+
+/** Test for {@link GenericInMemorySecretStore}. */
+public class GenericInMemorySecretStoreTest {
+
+private GenericInMemorySecretStore secretStore;
+
+@BeforeEach
+void setUp() {
+secretStore = new GenericInMemorySecretStore();
+}
+
+@Test
+void testStoreAndGetSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("username", "testuser");
+secretData.put("password", "testpass");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretId).isNotNull();
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret).isNotNull();
+assertThat(retrievedSecret.get("username")).isEqualTo("testuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("testpass");
+}
+
+@Test
+void testGetNonExistentSecret() {
+assertThatThrownBy(() -> secretStore.getSecret("non-existent-id"))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID 'non-existent-id' not
found");
+}
+
+@Test
+void testGetSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.getSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testStoreSecretWithNullData() {
+assertThatThrownBy(() -> secretStore.storeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret data cannot be null");
+}
+
+@Test
+void testRemoveSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("key", "value");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretStore.getSecret(secretId)).isNotNull();
+
+secretStore.removeSecret(secretId);
+assertThatThrownBy(() -> secretStore.getSecret(secretId))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID '" + secretId + "' not
found");
+}
+
+@Test
+void testRemoveSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.removeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testRemoveNonExistentSecret() {
+// Should not throw exception, just silently remove nothing
+secretStore.removeSecret("non-existent-id");
+}
+
+@Test
+void testUpdateSecret() throws SecretNotFoundException {
+Map originalData = new HashMap<>();
+originalData.put("username", "olduser");
+originalData.put("password", "oldpass");
+
+String secretId = secretStore.storeSecret(originalData);
+
+Map updatedData = new HashMap<>();
+updatedData.put("username", "newuser");
+updatedData.put("password", "newpass");
+
+secretStore.updateSecret(secretId, updatedData);
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret.get("username")).isEqualTo("newuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("newpass");
+}
+
+@Test
+void testUpdateNonExistentSecret() {
+Map secretData = new HashMap<>();
+secretData.put("key", "va
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738515926
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+
+/** Test for {@link GenericInMemorySecretStore}. */
+public class GenericInMemorySecretStoreTest {
+
+private GenericInMemorySecretStore secretStore;
+
+@BeforeEach
+void setUp() {
+secretStore = new GenericInMemorySecretStore();
+}
+
+@Test
+void testStoreAndGetSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("username", "testuser");
+secretData.put("password", "testpass");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretId).isNotNull();
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret).isNotNull();
+assertThat(retrievedSecret.get("username")).isEqualTo("testuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("testpass");
+}
+
+@Test
+void testGetNonExistentSecret() {
+assertThatThrownBy(() -> secretStore.getSecret("non-existent-id"))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID 'non-existent-id' not
found");
+}
+
+@Test
+void testGetSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.getSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testStoreSecretWithNullData() {
+assertThatThrownBy(() -> secretStore.storeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret data cannot be null");
+}
+
+@Test
+void testRemoveSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("key", "value");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretStore.getSecret(secretId)).isNotNull();
+
+secretStore.removeSecret(secretId);
+assertThatThrownBy(() -> secretStore.getSecret(secretId))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID '" + secretId + "' not
found");
+}
+
+@Test
+void testRemoveSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.removeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testRemoveNonExistentSecret() {
+// Should not throw exception, just silently remove nothing
+secretStore.removeSecret("non-existent-id");
+}
+
+@Test
+void testUpdateSecret() throws SecretNotFoundException {
+Map originalData = new HashMap<>();
+originalData.put("username", "olduser");
+originalData.put("password", "oldpass");
+
+String secretId = secretStore.storeSecret(originalData);
+
+Map updatedData = new HashMap<>();
+updatedData.put("username", "newuser");
+updatedData.put("password", "newpass");
+
+secretStore.updateSecret(secretId, updatedData);
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret.get("username")).isEqualTo("newuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("newpass");
+}
+
+@Test
+void testUpdateNonExistentSecret() {
+Map secretData = new HashMap<>();
+secretData.put("key", "va
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738506055
##
flink-table/flink-table-common/src/main/java/org/apache/flink/table/secret/exceptions/SecretException.java:
##
@@ -0,0 +1,61 @@
+/*
+ * 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.flink.table.secret.exceptions;
+
+import org.apache.flink.annotation.PublicEvolving;
+
+/**
+ * Base exception for all secret-related errors in the secret store.
+ *
+ * This exception serves as the parent class for all secret store related
exceptions, providing a
+ * common type for handling errors that occur during secret management
operations.
+ */
+@PublicEvolving
+public class SecretException extends RuntimeException {
Review Comment:
i wonder if it is
>for all secret store related exceptions,
should it be called then `SecretStoreException`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738506055
##
flink-table/flink-table-common/src/main/java/org/apache/flink/table/secret/exceptions/SecretException.java:
##
@@ -0,0 +1,61 @@
+/*
+ * 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.flink.table.secret.exceptions;
+
+import org.apache.flink.annotation.PublicEvolving;
+
+/**
+ * Base exception for all secret-related errors in the secret store.
+ *
+ * This exception serves as the parent class for all secret store related
exceptions, providing a
+ * common type for handling errors that occur during secret management
operations.
+ */
+@PublicEvolving
+public class SecretException extends RuntimeException {
Review Comment:
i wonder if it is is
>all secret store related exceptions,
should it be called then `SecretStoreException`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738498403
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+
+/** Test for {@link GenericInMemorySecretStore}. */
+public class GenericInMemorySecretStoreTest {
+
+private GenericInMemorySecretStore secretStore;
+
+@BeforeEach
+void setUp() {
+secretStore = new GenericInMemorySecretStore();
+}
+
+@Test
+void testStoreAndGetSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("username", "testuser");
+secretData.put("password", "testpass");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretId).isNotNull();
+
+Map retrievedSecret = secretStore.getSecret(secretId);
+assertThat(retrievedSecret).isNotNull();
+assertThat(retrievedSecret.get("username")).isEqualTo("testuser");
+assertThat(retrievedSecret.get("password")).isEqualTo("testpass");
+}
+
+@Test
+void testGetNonExistentSecret() {
+assertThatThrownBy(() -> secretStore.getSecret("non-existent-id"))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID 'non-existent-id' not
found");
+}
+
+@Test
+void testGetSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.getSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testStoreSecretWithNullData() {
+assertThatThrownBy(() -> secretStore.storeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret data cannot be null");
+}
+
+@Test
+void testRemoveSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("key", "value");
+
+String secretId = secretStore.storeSecret(secretData);
+assertThat(secretStore.getSecret(secretId)).isNotNull();
+
+secretStore.removeSecret(secretId);
+assertThatThrownBy(() -> secretStore.getSecret(secretId))
+.isInstanceOf(SecretNotFoundException.class)
+.hasMessageContaining("Secret with ID '" + secretId + "' not
found");
+}
+
+@Test
+void testRemoveSecretWithNullId() {
+assertThatThrownBy(() -> secretStore.removeSecret(null))
+.isInstanceOf(NullPointerException.class)
+.hasMessageContaining("Secret ID cannot be null");
+}
+
+@Test
+void testRemoveNonExistentSecret() {
+// Should not throw exception, just silently remove nothing
+secretStore.removeSecret("non-existent-id");
Review Comment:
```suggestion
assertDoesNotThrow(() ->
secretStore.removeSecret("non-existent-id"));
```
then just use `assertDoesNotThrow` instead of comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738491726
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+
+/** Test for {@link GenericInMemorySecretStore}. */
+public class GenericInMemorySecretStoreTest {
+
+private GenericInMemorySecretStore secretStore;
+
+@BeforeEach
+void setUp() {
+secretStore = new GenericInMemorySecretStore();
+}
+
+@Test
+void testStoreAndGetSecret() throws SecretNotFoundException {
+Map secretData = new HashMap<>();
+secretData.put("username", "testuser");
+secretData.put("password", "testpass");
Review Comment:
```suggestion
Map secretData = Map.of("username", "testuser",
"password", "testpass");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738489953
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreTest.java:
##
@@ -0,0 +1,202 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
+
+/** Test for {@link GenericInMemorySecretStore}. */
+public class GenericInMemorySecretStoreTest {
Review Comment:
```suggestion
class GenericInMemorySecretStoreTest {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738487943
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreFactoryTest.java:
##
@@ -0,0 +1,64 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.factories.FactoryUtil;
+import org.apache.flink.table.factories.FactoryUtil.DefaultSecretStoreContext;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+
+/** Test for {@link GenericInMemorySecretStoreFactory}. */
+public class GenericInMemorySecretStoreFactoryTest {
+
+@Test
+void testSecretStoreInit() {
+String factoryIdentifier =
GenericInMemorySecretStoreFactory.IDENTIFIER;
+Map options = new HashMap<>();
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+final DefaultSecretStoreContext discoveryContext =
+new DefaultSecretStoreContext(options, null, classLoader);
+final SecretStoreFactory factory =
+FactoryUtil.discoverFactory(
+classLoader, SecretStoreFactory.class,
factoryIdentifier);
+factory.open(discoveryContext);
+
+SecretStore secretStore = factory.createSecretStore();
+assertThat(secretStore instanceof GenericInMemorySecretStore).isTrue();
+
+factory.close();
+}
+
+@Test
+void testFactoryIdentifier() {
+GenericInMemorySecretStoreFactory factory = new
GenericInMemorySecretStoreFactory();
+assertThat(factory.factoryIdentifier()).isEqualTo("generic_in_memory");
+}
+
+@Test
+void testRequiredAndOptionalOptions() {
+GenericInMemorySecretStoreFactory factory = new
GenericInMemorySecretStoreFactory();
+assertThat(factory.requiredOptions().isEmpty()).isTrue();
+assertThat(factory.optionalOptions().isEmpty()).isTrue();
Review Comment:
```suggestion
assertThat(factory.requiredOptions()).isEmpty());
assertThat(factory.optionalOptions()).isEmpty());
```
will not it work like this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738484846
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreFactoryTest.java:
##
@@ -0,0 +1,64 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.factories.FactoryUtil;
+import org.apache.flink.table.factories.FactoryUtil.DefaultSecretStoreContext;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+
+/** Test for {@link GenericInMemorySecretStoreFactory}. */
+public class GenericInMemorySecretStoreFactoryTest {
+
+@Test
+void testSecretStoreInit() {
+String factoryIdentifier =
GenericInMemorySecretStoreFactory.IDENTIFIER;
+Map options = new HashMap<>();
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+final DefaultSecretStoreContext discoveryContext =
+new DefaultSecretStoreContext(options, null, classLoader);
+final SecretStoreFactory factory =
+FactoryUtil.discoverFactory(
+classLoader, SecretStoreFactory.class,
factoryIdentifier);
+factory.open(discoveryContext);
+
+SecretStore secretStore = factory.createSecretStore();
+assertThat(secretStore instanceof GenericInMemorySecretStore).isTrue();
+
+factory.close();
Review Comment:
should it be in try... catch...finally ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738481437
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/secret/GenericInMemorySecretStoreFactoryTest.java:
##
@@ -0,0 +1,64 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.table.factories.FactoryUtil;
+import org.apache.flink.table.factories.FactoryUtil.DefaultSecretStoreContext;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+
+/** Test for {@link GenericInMemorySecretStoreFactory}. */
+public class GenericInMemorySecretStoreFactoryTest {
Review Comment:
```suggestion
class GenericInMemorySecretStoreFactoryTest {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738480776
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/factories/TableFactoryUtilTest.java:
##
@@ -0,0 +1,222 @@
+/*
+ * 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.flink.table.factories;
+
+import org.apache.flink.configuration.Configuration;
+import org.apache.flink.table.catalog.CommonCatalogOptions;
+import org.apache.flink.table.catalog.FileCatalogStoreFactory;
+import org.apache.flink.table.catalog.GenericInMemoryCatalogStoreFactory;
+import org.apache.flink.table.secret.CommonSecretOptions;
+import org.apache.flink.table.secret.GenericInMemorySecretStoreFactory;
+import org.apache.flink.table.secret.SecretStoreFactory;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.File;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/** Tests for {@link TableFactoryUtil}. */
+class TableFactoryUtilTest {
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithGenericInMemory() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"generic_in_memory");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithFile() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+assertThat(factory).isInstanceOf(FileCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithDefaultKind() {
+Configuration configuration = new Configuration();
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testBuildCatalogStoreFactoryContext(@TempDir File tempFolder) {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+configuration.setString("table.catalog-store.file.path",
tempFolder.getAbsolutePath());
+configuration.setString("table.catalog-store.file.option1", "value1");
+configuration.setString("table.catalog-store.file.option2", "value2");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory.Context context =
+
TableFactoryUtil.buildCatalogStoreFactoryContext(configuration, classLoader);
+
+assertThat(context).isNotNull();
+assertThat(context.getOptions()).containsEntry("path",
tempFolder.getAbsolutePath());
+assertThat(context.getOptions()).containsEntry("option1", "value1");
+assertThat(context.getOptions()).containsEntry("option2", "value2");
+assertThat(context.getConfiguration()).isEqualTo(configuration);
+assertThat(context.getClassLoader()).isEqualTo(classLoader);
+}
+
+@Test
+void testBuildCatalogStoreFactoryContextWithGenericInMemory() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"generic_in_memory");
+
configuration.setString("table.catalog-store.generic_in_memory.option1",
"value1");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory.Context context =
+
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738480061
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/factories/TableFactoryUtilTest.java:
##
@@ -0,0 +1,222 @@
+/*
+ * 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.flink.table.factories;
+
+import org.apache.flink.configuration.Configuration;
+import org.apache.flink.table.catalog.CommonCatalogOptions;
+import org.apache.flink.table.catalog.FileCatalogStoreFactory;
+import org.apache.flink.table.catalog.GenericInMemoryCatalogStoreFactory;
+import org.apache.flink.table.secret.CommonSecretOptions;
+import org.apache.flink.table.secret.GenericInMemorySecretStoreFactory;
+import org.apache.flink.table.secret.SecretStoreFactory;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.File;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/** Tests for {@link TableFactoryUtil}. */
+class TableFactoryUtilTest {
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithGenericInMemory() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"generic_in_memory");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithFile() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+assertThat(factory).isInstanceOf(FileCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithDefaultKind() {
+Configuration configuration = new Configuration();
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testBuildCatalogStoreFactoryContext(@TempDir File tempFolder) {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+configuration.setString("table.catalog-store.file.path",
tempFolder.getAbsolutePath());
+configuration.setString("table.catalog-store.file.option1", "value1");
+configuration.setString("table.catalog-store.file.option2", "value2");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory.Context context =
+
TableFactoryUtil.buildCatalogStoreFactoryContext(configuration, classLoader);
+
+assertThat(context).isNotNull();
+assertThat(context.getOptions()).containsEntry("path",
tempFolder.getAbsolutePath());
+assertThat(context.getOptions()).containsEntry("option1", "value1");
+assertThat(context.getOptions()).containsEntry("option2", "value2");
+assertThat(context.getConfiguration()).isEqualTo(configuration);
+assertThat(context.getClassLoader()).isEqualTo(classLoader);
+}
+
+@Test
+void testBuildCatalogStoreFactoryContextWithGenericInMemory() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"generic_in_memory");
+
configuration.setString("table.catalog-store.generic_in_memory.option1",
"value1");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory.Context context =
+
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738477969
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/factories/TableFactoryUtilTest.java:
##
@@ -0,0 +1,222 @@
+/*
+ * 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.flink.table.factories;
+
+import org.apache.flink.configuration.Configuration;
+import org.apache.flink.table.catalog.CommonCatalogOptions;
+import org.apache.flink.table.catalog.FileCatalogStoreFactory;
+import org.apache.flink.table.catalog.GenericInMemoryCatalogStoreFactory;
+import org.apache.flink.table.secret.CommonSecretOptions;
+import org.apache.flink.table.secret.GenericInMemorySecretStoreFactory;
+import org.apache.flink.table.secret.SecretStoreFactory;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.File;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/** Tests for {@link TableFactoryUtil}. */
+class TableFactoryUtilTest {
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithGenericInMemory() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"generic_in_memory");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithFile() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+assertThat(factory).isInstanceOf(FileCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithDefaultKind() {
+Configuration configuration = new Configuration();
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testBuildCatalogStoreFactoryContext(@TempDir File tempFolder) {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+configuration.setString("table.catalog-store.file.path",
tempFolder.getAbsolutePath());
+configuration.setString("table.catalog-store.file.option1", "value1");
+configuration.setString("table.catalog-store.file.option2", "value2");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory.Context context =
+
TableFactoryUtil.buildCatalogStoreFactoryContext(configuration, classLoader);
+
+assertThat(context).isNotNull();
+assertThat(context.getOptions()).containsEntry("path",
tempFolder.getAbsolutePath());
+assertThat(context.getOptions()).containsEntry("option1", "value1");
+assertThat(context.getOptions()).containsEntry("option2", "value2");
+assertThat(context.getConfiguration()).isEqualTo(configuration);
+assertThat(context.getClassLoader()).isEqualTo(classLoader);
+}
+
+@Test
+void testBuildCatalogStoreFactoryContextWithGenericInMemory() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"generic_in_memory");
+
configuration.setString("table.catalog-store.generic_in_memory.option1",
"value1");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory.Context context =
+
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738473275
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/factories/TableFactoryUtilTest.java:
##
@@ -0,0 +1,222 @@
+/*
+ * 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.flink.table.factories;
+
+import org.apache.flink.configuration.Configuration;
+import org.apache.flink.table.catalog.CommonCatalogOptions;
+import org.apache.flink.table.catalog.FileCatalogStoreFactory;
+import org.apache.flink.table.catalog.GenericInMemoryCatalogStoreFactory;
+import org.apache.flink.table.secret.CommonSecretOptions;
+import org.apache.flink.table.secret.GenericInMemorySecretStoreFactory;
+import org.apache.flink.table.secret.SecretStoreFactory;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.File;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/** Tests for {@link TableFactoryUtil}. */
+class TableFactoryUtilTest {
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithGenericInMemory() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"generic_in_memory");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithFile() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+assertThat(factory).isInstanceOf(FileCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithDefaultKind() {
+Configuration configuration = new Configuration();
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
Review Comment:
We have these 3 lines in every test...
can we extract them into for instance `@BeforeEach` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738466762
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/factories/TableFactoryUtilTest.java:
##
@@ -0,0 +1,222 @@
+/*
+ * 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.flink.table.factories;
+
+import org.apache.flink.configuration.Configuration;
+import org.apache.flink.table.catalog.CommonCatalogOptions;
+import org.apache.flink.table.catalog.FileCatalogStoreFactory;
+import org.apache.flink.table.catalog.GenericInMemoryCatalogStoreFactory;
+import org.apache.flink.table.secret.CommonSecretOptions;
+import org.apache.flink.table.secret.GenericInMemorySecretStoreFactory;
+import org.apache.flink.table.secret.SecretStoreFactory;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.File;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/** Tests for {@link TableFactoryUtil}. */
+class TableFactoryUtilTest {
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithGenericInMemory() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"generic_in_memory");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithFile() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+assertThat(factory).isInstanceOf(FileCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithDefaultKind() {
+Configuration configuration = new Configuration();
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testBuildCatalogStoreFactoryContext(@TempDir File tempFolder) {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+configuration.setString("table.catalog-store.file.path",
tempFolder.getAbsolutePath());
+configuration.setString("table.catalog-store.file.option1", "value1");
+configuration.setString("table.catalog-store.file.option2", "value2");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory.Context context =
+
TableFactoryUtil.buildCatalogStoreFactoryContext(configuration, classLoader);
+
+assertThat(context).isNotNull();
+assertThat(context.getOptions()).containsEntry("path",
tempFolder.getAbsolutePath());
+assertThat(context.getOptions()).containsEntry("option1", "value1");
+assertThat(context.getOptions()).containsEntry("option2", "value2");
Review Comment:
or just
```java
assertThat(context.getOptions())
.containsAllEntriesOf(
Map.of("path", tempFolder.getAbsolutePath(), "option1", "value1", "option2",
"value2"));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738466762
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/factories/TableFactoryUtilTest.java:
##
@@ -0,0 +1,222 @@
+/*
+ * 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.flink.table.factories;
+
+import org.apache.flink.configuration.Configuration;
+import org.apache.flink.table.catalog.CommonCatalogOptions;
+import org.apache.flink.table.catalog.FileCatalogStoreFactory;
+import org.apache.flink.table.catalog.GenericInMemoryCatalogStoreFactory;
+import org.apache.flink.table.secret.CommonSecretOptions;
+import org.apache.flink.table.secret.GenericInMemorySecretStoreFactory;
+import org.apache.flink.table.secret.SecretStoreFactory;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.File;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/** Tests for {@link TableFactoryUtil}. */
+class TableFactoryUtilTest {
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithGenericInMemory() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"generic_in_memory");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithFile() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+assertThat(factory).isInstanceOf(FileCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithDefaultKind() {
+Configuration configuration = new Configuration();
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testBuildCatalogStoreFactoryContext(@TempDir File tempFolder) {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+configuration.setString("table.catalog-store.file.path",
tempFolder.getAbsolutePath());
+configuration.setString("table.catalog-store.file.option1", "value1");
+configuration.setString("table.catalog-store.file.option2", "value2");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory.Context context =
+
TableFactoryUtil.buildCatalogStoreFactoryContext(configuration, classLoader);
+
+assertThat(context).isNotNull();
+assertThat(context.getOptions()).containsEntry("path",
tempFolder.getAbsolutePath());
+assertThat(context.getOptions()).containsEntry("option1", "value1");
+assertThat(context.getOptions()).containsEntry("option2", "value2");
Review Comment:
or just
```java
assertThat(context.getOptions()).containsAllEntriesOf(Map.of("path",
tempFolder.getAbsolutePath(), "option1", "value1", "option2", "value2"))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738460520
##
flink-table/flink-table-api-java/src/test/java/org/apache/flink/table/factories/TableFactoryUtilTest.java:
##
@@ -0,0 +1,222 @@
+/*
+ * 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.flink.table.factories;
+
+import org.apache.flink.configuration.Configuration;
+import org.apache.flink.table.catalog.CommonCatalogOptions;
+import org.apache.flink.table.catalog.FileCatalogStoreFactory;
+import org.apache.flink.table.catalog.GenericInMemoryCatalogStoreFactory;
+import org.apache.flink.table.secret.CommonSecretOptions;
+import org.apache.flink.table.secret.GenericInMemorySecretStoreFactory;
+import org.apache.flink.table.secret.SecretStoreFactory;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import java.io.File;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/** Tests for {@link TableFactoryUtil}. */
+class TableFactoryUtilTest {
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithGenericInMemory() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"generic_in_memory");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithFile() {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+assertThat(factory).isInstanceOf(FileCatalogStoreFactory.class);
+}
+
+@Test
+void testFindAndCreateCatalogStoreFactoryWithDefaultKind() {
+Configuration configuration = new Configuration();
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory factory =
+
TableFactoryUtil.findAndCreateCatalogStoreFactory(configuration, classLoader);
+
+
assertThat(factory).isInstanceOf(GenericInMemoryCatalogStoreFactory.class);
+}
+
+@Test
+void testBuildCatalogStoreFactoryContext(@TempDir File tempFolder) {
+Configuration configuration = new Configuration();
+configuration.set(CommonCatalogOptions.TABLE_CATALOG_STORE_KIND,
"file");
+configuration.setString("table.catalog-store.file.path",
tempFolder.getAbsolutePath());
+configuration.setString("table.catalog-store.file.option1", "value1");
+configuration.setString("table.catalog-store.file.option2", "value2");
+ClassLoader classLoader =
Thread.currentThread().getContextClassLoader();
+
+CatalogStoreFactory.Context context =
+
TableFactoryUtil.buildCatalogStoreFactoryContext(configuration, classLoader);
+
+assertThat(context).isNotNull();
+assertThat(context.getOptions()).containsEntry("path",
tempFolder.getAbsolutePath());
+assertThat(context.getOptions()).containsEntry("option1", "value1");
+assertThat(context.getOptions()).containsEntry("option2", "value2");
Review Comment:
```suggestion
assertThat(context.getOptions())
.containsEntry("path", tempFolder.getAbsolutePath())
.containsEntry("option1", "value1")
.containsEntry("option2", "value2");
```
iirc it is chainable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738447209
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/secret/GenericInMemorySecretStoreFactory.java:
##
@@ -0,0 +1,65 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.configuration.ConfigOption;
+import org.apache.flink.table.catalog.exceptions.CatalogException;
+
+import java.util.Collections;
+import java.util.Set;
+
+/**
+ * Factory for creating {@link GenericInMemorySecretStore} instances.
+ *
+ * This factory creates in-memory secret stores that are suitable for
testing and development
+ * purposes. Secrets are stored in plaintext JSON format in memory and are not
persisted.
+ */
+@Internal
+public class GenericInMemorySecretStoreFactory implements SecretStoreFactory {
+
+private GenericInMemorySecretStore secretStore;
+public static final String IDENTIFIER = "generic_in_memory";
+
+@Override
+public String factoryIdentifier() {
+return IDENTIFIER;
+}
+
+@Override
+public Set> requiredOptions() {
+return Collections.emptySet();
+}
+
+@Override
+public Set> optionalOptions() {
+return Collections.emptySet();
Review Comment:
```suggestion
return Set.of();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738445192
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/secret/GenericInMemorySecretStoreFactory.java:
##
@@ -0,0 +1,65 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.configuration.ConfigOption;
+import org.apache.flink.table.catalog.exceptions.CatalogException;
+
+import java.util.Collections;
+import java.util.Set;
+
+/**
+ * Factory for creating {@link GenericInMemorySecretStore} instances.
+ *
+ * This factory creates in-memory secret stores that are suitable for
testing and development
+ * purposes. Secrets are stored in plaintext JSON format in memory and are not
persisted.
+ */
+@Internal
+public class GenericInMemorySecretStoreFactory implements SecretStoreFactory {
+
+private GenericInMemorySecretStore secretStore;
+public static final String IDENTIFIER = "generic_in_memory";
Review Comment:
usually `static` fields first, then non-`static`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738446301
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/secret/GenericInMemorySecretStoreFactory.java:
##
@@ -0,0 +1,65 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.configuration.ConfigOption;
+import org.apache.flink.table.catalog.exceptions.CatalogException;
+
+import java.util.Collections;
+import java.util.Set;
+
+/**
+ * Factory for creating {@link GenericInMemorySecretStore} instances.
+ *
+ * This factory creates in-memory secret stores that are suitable for
testing and development
+ * purposes. Secrets are stored in plaintext JSON format in memory and are not
persisted.
+ */
+@Internal
+public class GenericInMemorySecretStoreFactory implements SecretStoreFactory {
+
+private GenericInMemorySecretStore secretStore;
+public static final String IDENTIFIER = "generic_in_memory";
+
+@Override
+public String factoryIdentifier() {
+return IDENTIFIER;
+}
+
+@Override
+public Set> requiredOptions() {
+return Collections.emptySet();
Review Comment:
```suggestion
return Set.of();
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738440947
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/TableEnvironmentImpl.java:
##
@@ -263,6 +265,19 @@ public static TableEnvironmentImpl
create(EnvironmentSettings settings) {
? settings.getCatalogStore()
: catalogStoreFactory.createCatalogStore();
+final SecretStoreFactory secretStoreFactory =
+TableFactoryUtil.findAndCreateSecretStoreFactory(
+settings.getConfiguration(), userClassLoader);
+final SecretStoreFactory.Context secretStoreContext =
+TableFactoryUtil.buildSecretStoreFactoryContext(
+settings.getConfiguration(), userClassLoader);
+secretStoreFactory.open(secretStoreContext);
+// TODO: pass secret store to catalog manager for encryption/decryption
Review Comment:
Do we have a jira issue for that?
`TODO` are not trackable ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
snuyanzin commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2738435125
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/secret/GenericInMemorySecretStore.java:
##
@@ -0,0 +1,115 @@
+/*
+ * 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.flink.table.secret;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.table.secret.exceptions.SecretException;
+import org.apache.flink.table.secret.exceptions.SecretNotFoundException;
+
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.core.JsonProcessingException;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.core.type.TypeReference;
+import
org.apache.flink.shaded.jackson2.com.fasterxml.jackson.databind.ObjectMapper;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import static org.apache.flink.util.Preconditions.checkNotNull;
+
+/**
+ * A generic in-memory implementation of both {@link ReadableSecretStore} and
{@link
+ * WritableSecretStore}.
+ *
+ * This implementation stores secrets in memory as plaintext JSON strings.
It is suitable for
+ * testing and development purposes but should not be used in production
environments as secrets are
+ * not encrypted.
+ */
+@Internal
+public class GenericInMemorySecretStore implements ReadableSecretStore,
WritableSecretStore {
+
+private final Map secrets;
+private final ObjectMapper objectMapper;
Review Comment:
Based on javadoc for `ObjectMapper` it could be used as a singletone and
fully threadsafe.
Why then do we have one per object here?
https://github.com/FasterXML/jackson-databind/blob/c380d710a5173304345fe6ea2e3854503fc66192/src/main/java/com/fasterxml/jackson/databind/ObjectMapper.java#L83-L88
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
lihaosky commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2699412623
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/TableEnvironmentImpl.java:
##
@@ -263,6 +265,19 @@ public static TableEnvironmentImpl
create(EnvironmentSettings settings) {
? settings.getCatalogStore()
: catalogStoreFactory.createCatalogStore();
+final SecretStoreFactory secretStoreFactory =
Review Comment:
Yes. I added some tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
davidradl commented on code in PR #27394:
URL: https://github.com/apache/flink/pull/27394#discussion_r2676137731
##
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/internal/TableEnvironmentImpl.java:
##
@@ -263,6 +265,19 @@ public static TableEnvironmentImpl
create(EnvironmentSettings settings) {
? settings.getCatalogStore()
: catalogStoreFactory.createCatalogStore();
+final SecretStoreFactory secretStoreFactory =
Review Comment:
I suggest adding a test implementation of these interfaces to drive the
usage of the interfaces from tests, including writeable readable stores with
config options.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] [FLINK-38263][table] add secret store related interfaces [flink]
flinkbot commented on PR #27394: URL: https://github.com/apache/flink/pull/27394#issuecomment-3726349756 ## CI report: * 4ec5496924fb7fcb65c9e3556df1ee3888fb11d8 UNKNOWN Bot commands The @flinkbot bot supports the following commands: - `@flinkbot run azure` re-run the last Azure build -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
