joyhaldar commented on code in PR #14447: URL: https://github.com/apache/iceberg/pull/14447#discussion_r2523455045
########## bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryMetastoreCatalog.java: ########## @@ -63,6 +62,11 @@ public class BigQueryMetastoreCatalog extends BaseMetastoreCatalog public static final String GCP_LOCATION = "gcp.bigquery.location"; public static final String LIST_ALL_TABLES = "gcp.bigquery.list-all-tables"; + public static final String CLIENT_FACTORY = "gcp.bigquery.client.factory"; + private static final String GCS_IMPERSONATE_SERVICE_ACCOUNT = "gcs.impersonate.service-account"; + private static final String GCS_PROJECT_ID = "gcs.project-id"; Review Comment: Thank you so much for reviewing @rambleraptor. I wanted to explain the flow a bit: - [ImpersonatedBigQueryClientFactory](https://github.com/apache/iceberg/blob/81607aa4d1719f07f003100955d0bfe9863a57e9/bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/ImpersonatedBigQueryClientFactory.java) defines: `gcp.bigquery.impersonate.service-account` (used for BigQuery client) - [prepareFileIOProperties()](https://github.com/apache/iceberg/blob/81607aa4d1719f07f003100955d0bfe9863a57e9/bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryMetastoreCatalog.java#L167) transforms: `gcp.*` → `gcs.*` (for FileIO, based on constants defined in [GCPProperties](https://github.com/apache/iceberg/blob/81607aa4d1719f07f003100955d0bfe9863a57e9/gcp/src/main/java/org/apache/iceberg/gcp/GCPProperties.java#L53)) - GCPProperties reads: `[gcs.impersonate.service-account](https://github.com/apache/iceberg/blob/81607aa4d1719f07f003100955d0bfe9863a57e9/gcp/src/main/java/org/apache/iceberg/gcp/GCPProperties.java#L53)` Do you think this should be changed? I can try removing the private constants from BigQueryMetastoreCatalog and use GCPProperties.GCS_* directly. Please let me know what you think would be more appropriate. ########## bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryMetastoreCatalog.java: ########## @@ -63,6 +62,11 @@ public class BigQueryMetastoreCatalog extends BaseMetastoreCatalog public static final String GCP_LOCATION = "gcp.bigquery.location"; public static final String LIST_ALL_TABLES = "gcp.bigquery.list-all-tables"; + public static final String CLIENT_FACTORY = "gcp.bigquery.client.factory"; + private static final String GCS_IMPERSONATE_SERVICE_ACCOUNT = "gcs.impersonate.service-account"; + private static final String GCS_PROJECT_ID = "gcs.project-id"; Review Comment: Thank you so much for reviewing @rambleraptor. I wanted to explain the flow a bit: - [ImpersonatedBigQueryClientFactory](https://github.com/apache/iceberg/blob/81607aa4d1719f07f003100955d0bfe9863a57e9/bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/ImpersonatedBigQueryClientFactory.java) defines: `gcp.bigquery.impersonate.service-account` (used for BigQuery client) - [prepareFileIOProperties()](https://github.com/apache/iceberg/blob/81607aa4d1719f07f003100955d0bfe9863a57e9/bigquery/src/main/java/org/apache/iceberg/gcp/bigquery/BigQueryMetastoreCatalog.java#L167) transforms: `gcp.*` → `gcs.*` (for FileIO, based on constants defined in [GCPProperties](https://github.com/apache/iceberg/blob/81607aa4d1719f07f003100955d0bfe9863a57e9/gcp/src/main/java/org/apache/iceberg/gcp/GCPProperties.java#L53)) - (https://github.com/apache/iceberg/blob/81607aa4d1719f07f003100955d0bfe9863a57e9/gcp/src/main/java/org/apache/iceberg/gcp/GCPProperties.java#L53) reads: `gcs.impersonate.service-account` Do you think this should be changed? I can try removing the private constants from BigQueryMetastoreCatalog and use GCPProperties.GCS_* directly. Please let me know what you think would be more appropriate. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
