fix(quickstart): Correct Quickstart Instructions #1673
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adnanhemani
requested review from
MonkeyCanCode,
RussellSpitzer,
adutra,
ashvina,
collado-mike,
dennishuo,
dimas-b,
eric-maynard,
flyrain,
jackye1995,
jbonofre,
snazy,
takidau and
vvcephei
as code owners
adnanhemani
requested review from
HonahX,
ajantha-bhat,
ebyhr,
pingtimeout and
singhpk234
as code owners
Contributor
Author
flyrain
reviewed
May 24, 2025
Contributor
flyrain
left a comment
flyrain
left a comment
There was a problem hiding this comment.
Thanks @adnanhemani for the fix. LGTM overall. Left one minor comment.
site/content/in-dev/unreleased/getting-started/using-polaris.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Yufei Gu <[email protected]>
Contributor
Author
|
Missed that, thank you! |
singhpk234
reviewed
May 25, 2025
Contributor
singhpk234
left a comment
singhpk234
left a comment
There was a problem hiding this comment.
LGTM, just minor suggestion please make it Apache Polaris instead of just using Polaris.
| ``` | ||
|
|
||
| ## Next Steps | ||
| Congrats, you now have a running instance of Polaris! For further information regarding how to use Polaris, check out the [Using Polaris]({{% ref "using-polaris" %}}) page. |
Contributor
There was a problem hiding this comment.
Suggested change
| Congrats, you now have a running instance of Polaris! For further information regarding how to use Polaris, check out the [Using Polaris]({{% ref "using-polaris" %}}) page. | |
| Congrats, you now have a running instance of Apache Polaris! For further information regarding how to use Apache Polaris, check out the [Using Apache Polaris]({{% ref "using-polaris" %}}) page. |
Contributor
There was a problem hiding this comment.
You don't necessarily need Apache on every usage of Polaris. I do think this blurb is pretty verbose, though... information regarding how to use Polaris could be details on using Polaris or something.
Contributor
There was a problem hiding this comment.
+1 to what Eric said. The guideline is that, in a given document, the first occurrence of an Apache project should be "Apache XYZ", and the subsequent occurrences can be just "XYZ".
Considering that this is for the documentation website that contains "Apache Polaris" in the header, and that "Apache Polaris (incubating)" is mentioned in the very first page (Overview), we can just use "Polaris" in the rest of the documentation.
Contributor
Author
There was a problem hiding this comment.
Thanks all, kept the Apache part the same - but changed the language here.
pingtimeout
reviewed
May 27, 2025
| --no-build-cache | ||
|
|
||
| docker compose -f getting-started/jdbc/docker-compose-bootstrap-db.yml -f getting-started/jdbc/docker-compose.yml up -d | ||
| docker compose -p polaris -f getting-started/jdbc/docker-compose-bootstrap-db.yml -f getting-started/jdbc/docker-compose.yml up -d |
Contributor
There was a problem hiding this comment.
Given that the -p polaris parameter is used to start the containers, shouldn't it be used in the cleanup instructions as well? See the docker compose ... down from the associated .md file.
Contributor
Author
There was a problem hiding this comment.
Great catch! I've update it now, thanks!
sfc-gh-ygu
approved these changes
May 27, 2025
flyrain
previously approved these changes
May 27, 2025
github-project-automation
bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
flyrain
approved these changes
May 27, 2025
github-project-automation
bot
moved this from Ready to merge
to Done
in Basic Kanban Board
williamhyun
pushed a commit
to williamhyun/polaris
that referenced
this pull request
May 27, 2025
snazy
added a commit
to snazy/polaris
that referenced
this pull request
Jun 13, 2025
* fix: Remove duplicated code in IcebergCatalog (apache#1681) * Fix SparkClient listGenericTable to use ListGenericTablesRESTResponse ListTableResponse is the class used by iceberg endpoint, which is the same as ListGenericTablesRESTResponse. However, we are suppose to use ListGenericTablesRESTResponse to be correct * fix: Remove info log about deprecated internal method from PolarisConfiguration (apache#1672) Remove the INFO log about calls to this method in Polaris, because users cannot do anything about these messages. Phasing out old property names requires coordination with users (e.g. release notes), so it is not a matter of merely avoiding calls to that method in Polaris code. Fixes apache#1666 * Create a single binary distribution bundle (apache#1589) * fix(quickstart): Correct Quickstart Instructions (apache#1673) * Remove Java URI validations for Blob Storage providers (apache#1604) This is a retry at apache#1586, which I'll close if this PR is on the right direction instead. Java URI does not actually apply any normalization to URIs if we do not call URI.normalize() (which we currently do not). Additionally, blob storage providers like S3 and GCS can provide ".." and "." as valid fragments in URLs - which Java URI would attempt to normalize incorrectly. As a result, attempting to validate and/or normalize URIs for blob storage providers using the Java URI class is the incorrect behavior. While we may want to add location validation via regex later, removing it first should at least unblock the bug we see in apache#1545. * Improve test coverage for invalid inputs in Policy APIs (apache#1665) * Fix getting-started docker start by PR apache#1532 (apache#1687) * Fix the manual test broken by PR apache#1532 (apache#1688) * Fix credentials printing twice (apache#1682) * main: Update dependency com.diffplug.spotless:spotless-plugin-gradle to v7.0.4 (apache#1690) * INFO: last merged commit 493de03 --------- Co-authored-by: Alexandre Dutra <[email protected]> Co-authored-by: gh-yzou <[email protected]> Co-authored-by: Dmitri Bourlatchkov <[email protected]> Co-authored-by: Yufei Gu <[email protected]> Co-authored-by: Adnan Hemani <[email protected]> Co-authored-by: William Hyun <[email protected]> Co-authored-by: Christopher Lambert <[email protected]> Co-authored-by: Mend Renovate <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Making some changes to ensure that things work out of the box again with the instructions in Quickstart. Additionally, this changes the Quickstart over to using JDBC from EclipseLink.