[HOTFIX] FIX QUICKSTART #1646

singhpk234 · 2025-05-21T23:17:21Z

About the change

A couple of changes have went in, which has broken quick start experience

[1] This refactor missed jdbc : #1522 as it changed the common assets which are used outside of eclipse-link module.

I see pr for fixing this : #1614, I am happy if the author of the PR wants to take the 2nd part of change also in his pr and close this one, marking them as co-author for now, Thanks a lot for the contribution.

[2] The PR for Require explicit user-consent to enable HadoopFileIO went it which missed handling the quick start for all the persistences (eclipse-link / jdbc) and hence both of them can't be used.

I putting this PR out for reference, and would request if this could get merged as a hotfix, as presently none of the persistence is usable on main. Happy to close this PR as well if the @pjanuario wants to take the 2nd part of change also in.

Co-author @pjanuario

pjanuario · 2025-05-21T23:23:02Z

Awesome, thanks for picking this up. Honestly I am just starting to play with the tool and deep diving into it. I was also having troubles in launching an instance with jdbc and I just created a ticket for the first thing I know it was missing and the other I was still playing around to understand what was going on.
I am happy to close my pr favour of this one, no worries.

eric-maynard · 2025-05-21T23:30:01Z

getting-started/jdbc/docker-compose.yml

Do any other docker-compose files need this fix? For example getting-started/eclipselink/docker-compose.yml

I wonder why this format change is required even in this PR 🤔

I'm not sure this is required ... but i'm also not sure why the old version worked. Wouldn't the previous syntax have essentially been

LIST
[ "JAVA_DEBUG=true", "JAVA_DEBUG_PORT=*:5005", ...]

While this syntax is
OBJECT
{ JAVA_DEBUG => true, JAVA_DEBUG_PORT => *:5005 }

? YAML is always confusing to me

- is a list element in yaml. However, I looked at a couple docker-compose files in other projects and all have that - . It's not just yaml - it's yaml+docker-compose-syntax.

Anyway - whatever works and not break stuff if fine for me.

Do any other docker-compose files need this fix? For example getting-started/eclipselink/docker-compose.yml

yes, we do, added that !

I wonder why this format change is required even in this PR 🤔

I tried the following syntax, with the new variables and aparently it didn't work for me, didn't debug it further and hence for consistency moved everything back to the format that worked :

- POLARIS_FEATURES_ALLOW_INSECURE_STORAGE_TYPES=true - POLARIS_FEATURES_SUPPORTED_CATALOG_STORAGE_TYPES=[\"FILE\",\"S3\",\"GCS\",\"AZURE\"] - POLARIS_READINESS_IGNORE_SEVERE_ISSUES=true

? YAML is always confusing to me

same here, I am working on hit and trial basis here :), this worked and hence i moved, i can definetly take a deeper look !

eric-maynard

LGTM, but we should fix everywhere else that needs this

getting-started/jdbc/docker-compose.yml

dimas-b

Please rebase to get the latest CI changes.

dimas-b · 2025-05-22T14:34:26Z

getting-started/jdbc/docker-compose.yml

I wonder why this format change is required even in this PR 🤔

RussellSpitzer · 2025-05-22T14:46:42Z

getting-started/jdbc/docker-compose.yml

For another pr: Maybe this should only be FILE and we have a built in error message for users trying to use any other storage type to come back and redo their config. Not a huge deal.

Also we probably should not allow "ALLOW_INSECURE_STORAGE_TYPES" if an insecure storage type is not in supported storage types right? Also "ignore-severe-issues" probably shouldn't be allowed to be true unless a sever item has been selected.

Just thoughts for future cleanups though

Ack, I allowlisted all as this same script also used with cloud providers,.

Also we probably should not allow "ALLOW_INSECURE_STORAGE_TYPES" if an insecure storage type is not in supported storage types right? Also "ignore-severe-issues" probably shouldn't be allowed to be true unless a sever item has been selected.

Sure thing ! let me take a deeper look in this as fast followup

flyrain · 2025-05-22T17:20:51Z

LGTM, but we should fix everywhere else that needs this

Here is another issue broken by #1532, #1649. I agreed we need to fix them all. But not necessarily within this PR.

singhpk234 · 2025-05-22T17:40:26Z

Thank so much, everyone for the quick review (@flyrain @eric-maynard @dimas-b @RussellSpitzer @snazy) !

Thank you @pjanuario for your contribution (marked you as co-author) !

* fix(nightly-CI): Do not publish snapshots from forks (apache#1635) Adopt the `Nightly Build` workflow to not (try to) publish every night from forks. * main: Update dependency io.smallrye.config:smallrye-config-core to v3.13.0 (apache#1637) * Use echo to print script errors (apache#1648) * [HOTFIX] QUICKSTART (apache#1646) The change adds the following to fix Quick start experience : [1] ENV variables required by common assets after apache#1522 [2] New configs required to enable FILE based sources apache#1649 Co-authored-by: singhpk234 <singhpk234@users.noreply.github.com> Co-authored-by: pjanuario <pjanuario@users.noreply.github.com> * main: Update dependency gradle to v8.14.1 (apache#1652) * main: Update dependency gradle to v8.14.1 * Re-adopt PR to the project's needs --------- Co-authored-by: Robert Stupp <snazy@snazy.de> * [Policy Store] Add policyTypeCode to Slice/Index for Future Filtering Support and Update Policy Persistence Method (apache#1628) This PR adds policyTypeCode to the in-memory tree map store's slice and the SQL index on policy_mapping_records (already done in JDBC in apache#1468). This prepares for future features that need to filter efficiently by policy type, like listing all entities with a data compaction policy. It also updates the loadAllTargetsOnPolicy method to accept policyTypeCode, enabling it to use the new index for better performance. * fix(test): Do not let some more tests spam `/tmp` (apache#1651) * fix(test): Do not let some more tests not spam `/tmp` * `PolarisRestCatalogViewFileIntegrationTest` * `FileIOExceptionsTest` * `PolarisRestCatalogViewFileIntegrationTest` Changes the tests to leverage JUnit's `@TempDir`. Simplifies `PolarisEclipseLinkMetaStoreManagerTest` * review: rename the (now) abstract class * fix(testing): Do not let PolarisOverlappingTableTest spam `/tmp` (apache#1641) Changes the test to leverage JUnit's `@TempDir`. * Add CATALOG_MANAGE_METADATA to super privilege set of policy attachment privileges (apache#1643) * Fix quickstart doc with docker compose (apache#1610) * main: Update dependency boto3 to v1.38.22 (apache#1657) * Refactor IcebergCatalog to isolate internal state (apache#1659) Following up on apache#1694 * Restore `private` scope on internal fields in `IcebergCatalog` * Use a test-only setter instead of sub-classing to manage injecting test FileIO implementations * Refactor: Use per-request STS credentials (apache#1629) * Refactor: Use per-request STS credentials No functional changes. This is mostly to allow more storage integration flexibility in downstream build. This might also be useful for non-AWS storage. * fix and enforce more errorprone checks (apache#1663) enforces the following checks: https://errorprone.info/bugpattern/ObjectsHashCodePrimitive https://errorprone.info/bugpattern/OptionalMapToOptional https://errorprone.info/bugpattern/StringCharset https://errorprone.info/bugpattern/VariableNameSameAsType * Create a wrapper script to generate python client; regenerate the python client (apache#1347) As noted in apache#755 and elsewhere, the generated types in client/python are currently out of date. This introduces a script to regenerate them and a gradle task to run that script. I've also run the script, which necessitated several things to get tests passing: 1. There were small nonfunctional spec changes needed in order to keep the Python client working 2. The CLI and its tests required a few fixes to work with the updated Python client 3. Many of the regtests required fixes to work with the updated Python client * [Python Client] CI for Python client (Continue PR#1096) (apache#1639) Adds CI for python client. It does not include caching poetry step for now since we do not have poetry.lock (it is in .gitignore), see relevant discussion in: apache#1102 (comment), apache#1096 (comment), we can add that later * main: Update actions/setup-python action to v5 (apache#1671) * main: Update actions/checkout action to v4 (apache#1670) * main: Update python Docker tag to v3.13 (apache#1669) * main: Update dependency pytest to ~=7.4.4 (apache#1668) * main: Update dependency software.amazon.awssdk:bom to v2.31.50 (apache#1677) * main: Update dependency boto3 to v1.38.23 (apache#1667) * feat(build): make archive builds reproducible (apache#1664) See https://docs.gradle.org/current/userguide/working_with_files.html#sec:reproducible_archives * main: Update dependency io.prometheus:prometheus-metrics-exporter-servlet-jakarta to v1.3.8 (apache#1679) * NoSQL: adapt to change on oss/main * INFO: Last merged commit: 6ef8b3e --------- Co-authored-by: Mend Renovate <bot@renovateapp.com> Co-authored-by: ModEtchFill <50123102+ModEtchFill@users.noreply.github.com> Co-authored-by: Prashant Singh <35593236+singhpk234@users.noreply.github.com> Co-authored-by: singhpk234 <singhpk234@users.noreply.github.com> Co-authored-by: pjanuario <pjanuario@users.noreply.github.com> Co-authored-by: Honah (Jonas) J. <honahx@apache.org> Co-authored-by: MonkeyCanCode <yongzheng0809@gmail.com> Co-authored-by: Dmitri Bourlatchkov <dmitri.bourlatchkov@dremio.com> Co-authored-by: Dmitri Bourlatchkov <dmitri.bourlatchkov@gmail.com> Co-authored-by: Christopher Lambert <xn137@gmx.de> Co-authored-by: Eric Maynard <eric.maynard+oss@snowflake.com>

singhpk234 requested review from HonahX, MonkeyCanCode, RussellSpitzer, adutra, ajantha-bhat, ashvina, collado-mike, dennishuo, dimas-b, ebyhr, eric-maynard, flyrain, jackye1995, jbonofre, pingtimeout, snazy, takidau and vvcephei as code owners May 21, 2025 23:17

github-project-automation bot moved this to PRs In Progress in Basic Kanban Board May 21, 2025

github-project-automation bot added this to Basic Kanban Board May 21, 2025

eric-maynard reviewed May 21, 2025

View reviewed changes

eric-maynard previously approved these changes May 21, 2025

View reviewed changes

github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board May 21, 2025

flyrain previously approved these changes May 22, 2025

View reviewed changes

getting-started/jdbc/docker-compose.yml Outdated Show resolved Hide resolved

pjanuario mentioned this pull request May 22, 2025

fix(quickstart): fixed quickstart jdbc to include client id and secrets (#1613) #1614

Closed

dimas-b reviewed May 22, 2025

View reviewed changes

getting-started/jdbc/docker-compose.yml Outdated

Copy link

Contributor

dimas-b May 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why this format change is required even in this PR 🤔

RussellSpitzer reviewed May 22, 2025

View reviewed changes

singhpk234 added 2 commits May 22, 2025 09:59

[HOTFIX] FIX JDBC QUICKSTART

f9083e9

Address review feedbacks

97c684f

singhpk234 dismissed stale reviews from flyrain and eric-maynard via 97c684f May 22, 2025 17:11

singhpk234 force-pushed the hotfix/jdbc-fix branch from 746e389 to 97c684f Compare May 22, 2025 17:11

flyrain approved these changes May 22, 2025

View reviewed changes

singhpk234 merged commit d619bf3 into apache:main May 22, 2025
6 checks passed

github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board May 22, 2025

singhpk234 changed the title ~~[HOTFIX] FIX JDBC QUICKSTART~~ [HOTFIX] FIX QUICKSTART May 22, 2025

singhpk234 mentioned this pull request May 22, 2025

Quick start broken #1647

Closed

adnanhemani mentioned this pull request May 22, 2025

Quickstart getting-started/jdbc is missing Client Id and Client Secret #1613

Closed

adnanhemani mentioned this pull request May 22, 2025

Fix quickstart doc with docker compose #1610

Merged

[HOTFIX] FIX QUICKSTART #1646

[HOTFIX] FIX QUICKSTART #1646

Uh oh!

Conversation

singhpk234 commented May 21, 2025

About the change

Uh oh!

pjanuario commented May 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-maynard left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dimas-b left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flyrain commented May 22, 2025

Uh oh!

Uh oh!

singhpk234 commented May 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants