dimas-b commented on code in PR #4627:
URL: https://github.com/apache/polaris/pull/4627#discussion_r3383078562
##########
site/content/in-dev/unreleased/getting-started/setup-tools.md:
##########
@@ -37,7 +37,7 @@ This command supports bootstrapping new environments and
exporting existing conf
If you already have an Apache Polaris environment, you can export its current
state to a YAML file using the `export` subcommand:
```bash
-polaris setup export > polaris_bootstrap.yaml
+polaris setup export --client-id ${CLIENT_ID} --client-secret ${CLIENT_SECRET}
> polaris_bootstrap.yaml
Review Comment:
Does adding `--client-id` / `--client-secret` options alone help users? This
page does not talk about how these credentials can be obtained 🤔
##########
site/content/guides/jdbc/docker-compose.yml:
##########
@@ -102,6 +104,8 @@ services:
condition: service_completed_successfully
stdin_open: true
tty: true
+ extra_hosts:
Review Comment:
nit: it might be easier to review / merge if we did doc changes and
docker-compose changes in separate PRs 🤔 As for me, I'm still not sure how
users can benefit from this particular change because none of the guides have
any instructions that use these new host names 🤔
##########
site/content/in-dev/unreleased/getting-started/using-polaris/_index.md:
##########
@@ -150,15 +150,15 @@ _Note: the credentials provided here are those for our
principal, not the root c
bin/spark-sql \
--packages
org.apache.iceberg:iceberg-spark-runtime-3.5_2.12:1.10.0,org.apache.iceberg:iceberg-aws-bundle:1.10.0
\
--conf
spark.sql.extensions=org.apache.iceberg.spark.extensions.IcebergSparkSessionExtensions
\
---conf spark.sql.catalog.quickstart_catalog.warehouse=quickstart_catalog \
---conf
spark.sql.catalog.quickstart_catalog.header.X-Iceberg-Access-Delegation=vended-credentials
\
---conf
spark.sql.catalog.quickstart_catalog=org.apache.iceberg.spark.SparkCatalog \
---conf
spark.sql.catalog.quickstart_catalog.catalog-impl=org.apache.iceberg.rest.RESTCatalog
\
---conf
spark.sql.catalog.quickstart_catalog.uri=http://localhost:8181/api/catalog \
---conf
spark.sql.catalog.quickstart_catalog.credential=${USER_CLIENT_ID}:${USER_CLIENT_SECRET}
\
---conf spark.sql.catalog.quickstart_catalog.scope='PRINCIPAL_ROLE:ALL' \
---conf spark.sql.catalog.quickstart_catalog.token-refresh-enabled=true \
---conf spark.sql.catalog.quickstart_catalog.client.region=us-west-2
+--conf spark.sql.catalog.polaris.warehouse=quickstart_catalog \
Review Comment:
This change is not strictly necessary. The `quickstart_catalog` property
name is relevant to the Spark SQl session (below), so it's local to this page
(the name can be anything).
That said, +1 to using `polaris` as the Spark catalog name.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
