Skip to content

[HOTFIX] FIX QUICKSTART #1646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 22, 2025
Merged

[HOTFIX] FIX QUICKSTART #1646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f9083e9
[HOTFIX] FIX JDBC QUICKSTART
singhpk234 May 21, 2025
97c684f
Address review feedbacks
singhpk234 May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions getting-started/eclipselink/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ services:
polaris.realm-context.realms: POLARIS
quarkus.otel.sdk.disabled: "true"
POLARIS_BOOTSTRAP_CREDENTIALS: POLARIS,${CLIENT_ID},${CLIENT_SECRET}
polaris.features."ALLOW_INSECURE_STORAGE_TYPES": "true"
polaris.features."SUPPORTED_CATALOG_STORAGE_TYPES": "[\"FILE\",\"S3\",\"GCS\",\"AZURE\"]"
polaris.readiness.ignore-severe-issues: "true"
volumes:
- ../assets/eclipselink/:/deployments/config/eclipselink
healthcheck:
Expand Down
2 changes: 2 additions & 0 deletions getting-started/jdbc/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ This example requires `jq` to be installed on your machine.
export QUARKUS_DATASOURCE_JDBC_URL=jdbc:postgresql://postgres:5432/POLARIS
export QUARKUS_DATASOURCE_USERNAME=postgres
export QUARKUS_DATASOURCE_PASSWORD=postgres
export CLIENT_ID=root
export CLIENT_SECRET=s3cr3t
docker compose -f getting-started/jdbc/docker-compose-bootstrap-db.yml -f getting-started/assets/postgres/docker-compose-postgres.yml -f getting-started/jdbc/docker-compose.yml up
```

Expand Down
33 changes: 21 additions & 12 deletions getting-started/jdbc/docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,22 @@ services:
# Optional, allows attaching a debugger to the Polaris JVM
- "5005:5005"
environment:
- JAVA_DEBUG=true
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor

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 🤔

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

- 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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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 !

- JAVA_DEBUG_PORT=*:5005
- POLARIS_PERSISTENCE_TYPE=relational-jdbc
- POLARIS_PERSISTENCE_RELATIONAL_JDBC_MAX_RETRIES=5
- POLARIS_PERSISTENCE_RELATIONAL_JDBC_INITIAL_DELAY_IN_MS=100
- POLARIS_PERSISTENCE_RELATIONAL_JDBC_MAX_DELAY_IN_MS=5000
- QUARKUS_DATASOURCE_DB_KIND=pgsql
- QUARKUS_DATASOURCE_JDBC_URL=${QUARKUS_DATASOURCE_JDBC_URL}
- QUARKUS_DATASOURCE_USERNAME=${QUARKUS_DATASOURCE_USERNAME}
- QUARKUS_DATASOURCE_PASSWORD=${QUARKUS_DATASOURCE_PASSWORD}
- POLARIS_REALM_CONTEXT_REALMS=POLARIS
- QUARKUS_OTEL_SDK_DISABLED=true
JAVA_DEBUG: true
JAVA_DEBUG_PORT: "*:5005"
POLARIS_PERSISTENCE_TYPE: relational-jdbc
POLARIS_PERSISTENCE_RELATIONAL_JDBC_MAX_RETRIES: 5
POLARIS_PERSISTENCE_RELATIONAL_JDBC_INITIAL_DELAY_IN_MS: 100
POLARIS_PERSISTENCE_RELATIONAL_JDBC_MAX_DELAY_IN_MS: 5000
QUARKUS_DATASOURCE_DB_KIND: pgsql
QUARKUS_DATASOURCE_JDBC_URL: $QUARKUS_DATASOURCE_JDBC_URL
QUARKUS_DATASOURCE_USERNAME: $QUARKUS_DATASOURCE_USERNAME
QUARKUS_DATASOURCE_PASSWORD: $QUARKUS_DATASOURCE_PASSWORD
POLARIS_REALM_CONTEXT_REALMS: POLARIS
QUARKUS_OTEL_SDK_DISABLED: true
POLARIS_BOOTSTRAP_CREDENTIALS: POLARIS,$CLIENT_ID,$CLIENT_SECRET
polaris.features."ALLOW_INSECURE_STORAGE_TYPES": "true"
polaris.features."SUPPORTED_CATALOG_STORAGE_TYPES": "[\"FILE\",\"S3\",\"GCS\",\"AZURE\"]"
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

polaris.readiness.ignore-severe-issues: "true"
healthcheck:
test: ["CMD", "curl", "http://localhost:8182/q/health"]
interval: 2s
Expand All @@ -57,6 +61,8 @@ services:
- STORAGE_LOCATION=${STORAGE_LOCATION}
- AWS_ROLE_ARN=${AWS_ROLE_ARN}
- AZURE_TENANT_ID=${AZURE_TENANT_ID}
- CLIENT_ID=${CLIENT_ID}
- CLIENT_SECRET=${CLIENT_SECRET}
volumes:
- ../assets/polaris/:/polaris
entrypoint: '/bin/sh -c "chmod +x /polaris/create-catalog.sh && /polaris/create-catalog.sh"'
Expand Down Expand Up @@ -98,5 +104,8 @@ services:
tty: true
ports:
- "8080:8080"
environment:
- CLIENT_ID=${CLIENT_ID}
- CLIENT_SECRET=${CLIENT_SECRET}
volumes:
- ../assets/trino-config/catalog:/etc/trino/catalog
Loading