Re: [PR] [#5355] export Paimon jdbc user and password to properites [gravitino]
mchades commented on PR #5376: URL: https://github.com/apache/gravitino/pull/5376#issuecomment-2449096603 > should we mask the jdbc user and password on the gravitino ui? I think need. @LauraXia123 cc -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
jerryshao merged PR #5398: URL: https://github.com/apache/gravitino/pull/5398 -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
mchades merged PR #5376: URL: https://github.com/apache/gravitino/pull/5376 -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
jerryshao closed pull request #5398: [#5355] export Paimon jdbc user and password to properites URL: https://github.com/apache/gravitino/pull/5398 -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
mchades commented on code in PR #5376: URL: https://github.com/apache/gravitino/pull/5376#discussion_r1824025203 ## catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java: ## @@ -128,13 +128,13 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada "Gravitino Paimon catalog jdbc user", false /* immutable */, null /* defaultValue */, -true /* hidden */), +false /* hidden */), Review Comment: ok -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
caican00 commented on code in PR #5376: URL: https://github.com/apache/gravitino/pull/5376#discussion_r1823892708 ## catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java: ## @@ -128,13 +128,13 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada "Gravitino Paimon catalog jdbc user", false /* immutable */, null /* defaultValue */, -true /* hidden */), +false /* hidden */), Review Comment: i checked the MysqlCatalog, PGCatalog, and tec. None of them tested whether the jdbc user and password were hidden or not. I think this pr can not include the test of this modification, but a new pr can be created to specifically improve them. WDYT? cc @mchades @FANNG1 -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
LauraXia123 commented on PR #5376: URL: https://github.com/apache/gravitino/pull/5376#issuecomment-2449182061 > > should we mask the jdbc user and password on the gravitino ui? > > I think need. @LauraXia123 cc https://github.com/user-attachments/assets/00cefc34-40f3-485e-a21d-70a4d9165433";> the password is masked on ui -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
caican00 commented on code in PR #5376: URL: https://github.com/apache/gravitino/pull/5376#discussion_r1823892708 ## catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java: ## @@ -128,13 +128,13 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada "Gravitino Paimon catalog jdbc user", false /* immutable */, null /* defaultValue */, -true /* hidden */), +false /* hidden */), Review Comment: i checked the MysqlCatalog, PGCatalog, and tec. None of them tested whether the jdbc user and password were hidden or not. I think this pr can not include the test of this modification, but a new pr can be created to specifically improve them -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
caican00 commented on code in PR #5376: URL: https://github.com/apache/gravitino/pull/5376#discussion_r1823892708 ## catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java: ## @@ -128,13 +128,13 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada "Gravitino Paimon catalog jdbc user", false /* immutable */, null /* defaultValue */, -true /* hidden */), +false /* hidden */), Review Comment: i checked the MysqlCatalog, PGCatalog, and tec. None of them tested whether the jdbc user and password were hidden or not. I think it is ok this pr can not include the test of this modification, but a new pr can be created to specifically improve them -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
mchades commented on code in PR #5376: URL: https://github.com/apache/gravitino/pull/5376#discussion_r1823828156 ## catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java: ## @@ -128,13 +128,13 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada "Gravitino Paimon catalog jdbc user", false /* immutable */, null /* defaultValue */, -true /* hidden */), +false /* hidden */), Review Comment: Is there a Paimon-JDBC catalog IT? If so, it would be easy to add the properties assert tests. Otherwise, we should supplement the IT. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] [#5355] export Paimon jdbc user and password to properites [gravitino]
FANNG1 commented on code in PR #5376: URL: https://github.com/apache/gravitino/pull/5376#discussion_r1823830897 ## catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java: ## @@ -128,13 +128,13 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada "Gravitino Paimon catalog jdbc user", false /* immutable */, null /* defaultValue */, -true /* hidden */), +false /* hidden */), Review Comment: Not the mater of easy or not, I think there is no need to add this to IT, since it's minor changes -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
FANNG1 commented on code in PR #5376: URL: https://github.com/apache/gravitino/pull/5376#discussion_r1823830897 ## catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java: ## @@ -128,13 +128,13 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada "Gravitino Paimon catalog jdbc user", false /* immutable */, null /* defaultValue */, -true /* hidden */), +false /* hidden */), Review Comment: I think there is no need to add this to IT, since it's minor changes -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
caican00 commented on PR #5376: URL: https://github.com/apache/gravitino/pull/5376#issuecomment-2449089901 should we mask the jdbc user and password on the gravitino ui? -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
FANNG1 commented on code in PR #5376: URL: https://github.com/apache/gravitino/pull/5376#discussion_r1823740787 ## catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java: ## @@ -128,13 +128,13 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada "Gravitino Paimon catalog jdbc user", false /* immutable */, null /* defaultValue */, -true /* hidden */), +false /* hidden */), Review Comment: no, is it nessecery to add the specific test for Paimon JDBC catalog hidden properties? -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
mchades commented on code in PR #5376: URL: https://github.com/apache/gravitino/pull/5376#discussion_r1823711373 ## catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonCatalogPropertiesMetadata.java: ## @@ -128,13 +128,13 @@ public class PaimonCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada "Gravitino Paimon catalog jdbc user", false /* immutable */, null /* defaultValue */, -true /* hidden */), +false /* hidden */), Review Comment: Is there any test to cover this change? -- 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] [#5355] export Paimon jdbc user and password to properites [gravitino]
FANNG1 commented on PR #5376: URL: https://github.com/apache/gravitino/pull/5376#issuecomment-2448909523 @mchades please help to review, thanks -- 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]
