You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "olsoloviov (via GitHub)" <gi...@apache.org> on 2025/09/26 14:13:12 UTC
[PR] Docs/improve idp documentation [polaris]
olsoloviov opened a new pull request, #2695:
URL: https://github.com/apache/polaris/pull/2695
<!--
Possible security vulnerabilities: STOP here and contact security@apache.org instead!
Please update the title of the PR with a meaningful message - do not leave it "empty" or "generated"
Please update this summary field:
The summary should cover these topics, if applicable:
* the motivation for the change
* a description of the status quo, for example the current behavior
* the desired behavior
* etc
PR checklist:
- Do a self-review of your code before opening a pull request
- Make sure that there's good test coverage for the changes included in this PR
- Run tests locally before pushing a PR (./gradlew check)
- Code should have comments where applicable. Particularly hard-to-understand
areas deserve good in-line documentation.
- Include changes and enhancements to the documentation (in site/content/in-dev/unreleased)
- For Work In Progress Pull Requests, please use the Draft PR feature.
Make sure to add the information BELOW this comment.
Everything in this comment will NOT be added to the PR description.
-->
Some improvements in documentation, mainly for IDP integration and telemetry
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Docs/improve idp documentation [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #2695:
URL: https://github.com/apache/polaris/pull/2695#discussion_r2383670490
##########
client/python/cli/command/__init__.py:
##########
@@ -186,7 +186,5 @@ def validate(self) -> None:
"""
Used to validate a command. Should always be called before `execute`. The arg parser will catch many issues
with options, but this is used to apply additional constraints that the arg parser can't currently handle.
- One example is that a catalog cannot be created with the `s3` storage type without a `--role-arn` option, but
Review Comment:
In that case, we are fine here. Sorry, I didn't notice #2339 has removed the check completely.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Docs/improve idp documentation [polaris]
Posted by "olsoloviov (via GitHub)" <gi...@apache.org>.
olsoloviov commented on code in PR #2695:
URL: https://github.com/apache/polaris/pull/2695#discussion_r2383655288
##########
client/python/cli/command/__init__.py:
##########
@@ -186,7 +186,5 @@ def validate(self) -> None:
"""
Used to validate a command. Should always be called before `execute`. The arg parser will catch many issues
with options, but this is used to apply additional constraints that the arg parser can't currently handle.
- One example is that a catalog cannot be created with the `s3` storage type without a `--role-arn` option, but
Review Comment:
Thanks for your review, @flyrain! I removed that comment since the actual validation of role-arn for S3 storage has been removed from _catalogs_ command in #2339, so the comment might be misleading now. We could specify AWS S3 but there is no specific validation for AWS S3 as well. I can replace it with a different relevant example if that makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Docs/improve idp documentation [polaris]
Posted by "olsoloviov (via GitHub)" <gi...@apache.org>.
olsoloviov commented on code in PR #2695:
URL: https://github.com/apache/polaris/pull/2695#discussion_r2382906790
##########
site/content/in-dev/unreleased/getting-started/using-polaris/keycloak-idp.md:
##########
@@ -48,7 +50,7 @@ Polaris is configured with 3 realms:
issued by both Polaris and Keycloak.
For more information about how to configure Polaris with external authentication, see the
-[Polaris documentation]({{% ref "../external-idp" %}}).
+[Polaris documentation](https://polaris.apache.org/in-dev/unreleased/managing-security/external-idp/).
Review Comment:
Thanks for catching!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Docs/improve idp documentation [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #2695:
URL: https://github.com/apache/polaris/pull/2695#discussion_r2383534094
##########
client/python/cli/command/__init__.py:
##########
@@ -186,7 +186,5 @@ def validate(self) -> None:
"""
Used to validate a command. Should always be called before `execute`. The arg parser will catch many issues
with options, but this is used to apply additional constraints that the arg parser can't currently handle.
- One example is that a catalog cannot be created with the `s3` storage type without a `--role-arn` option, but
Review Comment:
Why do we delete this? We could say that `AWS s3` to be more specific if `s3` is too board here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Docs/improve idp documentation [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b merged PR #2695:
URL: https://github.com/apache/polaris/pull/2695
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Docs/improve idp documentation [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #2695:
URL: https://github.com/apache/polaris/pull/2695#discussion_r2382775143
##########
site/content/in-dev/unreleased/command-line-interface.md:
##########
@@ -133,8 +133,12 @@ options:
--type The type of catalog to create in [INTERNAL, EXTERNAL]. INTERNAL by default.
--storage-type (Required) The type of storage to use for the catalog
--default-base-location (Required) Default base location of the catalog
+ --endpoint (Only for S3) The S3 endpoint to use when connecting to S3
+ --endpoint-internal (Only for S3) The S3 endpoint used by Polaris to use when connecting to S3, if different from the one that clients use
+ --sts-endpoint (Only for S3) The STS endpoint to use when connecting to STS
+ --path-style-access (Only for S3) Whether to use path-style-access for S3
--allowed-location An allowed location for files tracked by the catalog. Multiple locations can be provided by specifying this option more than once.
- --role-arn (Required for S3) A role ARN to use when connecting to S3
+ --role-arn (Only for S3) A role ARN to use when connecting to S3
Review Comment:
Only for AWS
##########
site/content/in-dev/unreleased/getting-started/creating-a-catalog/s3/_index.md:
##########
@@ -32,7 +32,7 @@ For the `polaris catalogs create` [command]({{% ref "../../../command-line-inter
```text
--storage-type s3
---role-arn (Required for S3) A role ARN to use when connecting to S3
+--role-arn (Only for S3) A role ARN to use when connecting to S3
Review Comment:
It's required only for AWS. MinIO does not need it. I _guess_ other non-AWS systems do not need it either.
##########
site/content/in-dev/unreleased/getting-started/using-polaris/keycloak-idp.md:
##########
@@ -48,7 +50,7 @@ Polaris is configured with 3 realms:
issued by both Polaris and Keycloak.
For more information about how to configure Polaris with external authentication, see the
-[Polaris documentation]({{% ref "../external-idp" %}}).
+[Polaris documentation](https://polaris.apache.org/in-dev/unreleased/managing-security/external-idp/).
Review Comment:
Can this be a "hugo" `ref`? Docs will be "frozen" after a release to links to `in-dev` from old versions are likely to become invalid pretty soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Docs/improve idp documentation [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #2695:
URL: https://github.com/apache/polaris/pull/2695#discussion_r2382765627
##########
site/content/in-dev/unreleased/getting-started/using-polaris/keycloak-idp.md:
##########
@@ -48,7 +50,7 @@ Polaris is configured with 3 realms:
issued by both Polaris and Keycloak.
For more information about how to configure Polaris with external authentication, see the
-[Polaris documentation]({{% ref "../external-idp" %}}).
+[Polaris documentation](https://polaris.apache.org/in-dev/unreleased/managing-security/external-idp/).
Review Comment:
Can this be a "hugo" `ref`? Docs will be "frozen" after a release, so links to `in-dev` from old versions are likely to become invalid pretty soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Docs/improve idp documentation [polaris]
Posted by "olsoloviov (via GitHub)" <gi...@apache.org>.
olsoloviov commented on code in PR #2695:
URL: https://github.com/apache/polaris/pull/2695#discussion_r2382908686
##########
site/content/in-dev/unreleased/command-line-interface.md:
##########
@@ -133,8 +133,12 @@ options:
--type The type of catalog to create in [INTERNAL, EXTERNAL]. INTERNAL by default.
--storage-type (Required) The type of storage to use for the catalog
--default-base-location (Required) Default base location of the catalog
+ --endpoint (Only for S3) The S3 endpoint to use when connecting to S3
+ --endpoint-internal (Only for S3) The S3 endpoint used by Polaris to use when connecting to S3, if different from the one that clients use
+ --sts-endpoint (Only for S3) The STS endpoint to use when connecting to STS
+ --path-style-access (Only for S3) Whether to use path-style-access for S3
--allowed-location An allowed location for files tracked by the catalog. Multiple locations can be provided by specifying this option more than once.
- --role-arn (Required for S3) A role ARN to use when connecting to S3
+ --role-arn (Only for S3) A role ARN to use when connecting to S3
Review Comment:
Thanks, fixed. Since it's a dumped output from CLI tool, I changed corresponding Python files as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org