You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "snazy (via GitHub)" <gi...@apache.org> on 2026/01/29 14:31:21 UTC
[PR] Guides: fix setup scripts to yield correct exit code [polaris]
snazy opened a new pull request, #3612:
URL: https://github.com/apache/polaris/pull/3612
The statements in the shell scripts for the setup services are often concatenated using `;`, which means that a previous' command exit code is _not_ propagated and the service, although it failed, is determined to be successful.
This change updates those scripts to use `&&` for the statement concatenation.
"Final" setup services (aka "polaris-setup") now have a final `sleep 120`. This is due to the behavior of `docker compose up --detach --wait`, which considers _any_ service (without dependants) that exits with exit code 0 as a failure, leading to that docker-compose command yielding an error code. That would break the guides testing code (#3553). That `sleep 120` in "polaris-setup" services does **not** cause a delay of the compose starting up - it is purely a "hack around" Docker Compose not having a notion of "setup services".
To avoid merge conflicts, this change also:
* updates affected `curl` invocations (as #3610)
* removes superfluous `restart: "no"`
--
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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on PR #3612:
URL: https://github.com/apache/polaris/pull/3612#issuecomment-3840494519
@MonkeyCanCode mind taking another look?
--
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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3612:
URL: https://github.com/apache/polaris/pull/3612#discussion_r2745406717
##########
getting-started/keycloak/docker-compose.yml:
##########
@@ -71,12 +71,12 @@ services:
command:
- "-c"
- >-
- apk add --no-cache jq &&
- chmod +x /polaris/create-catalog.sh &&
+ apk add --no-cache jq &&
token=$$(curl http://keycloak:8080/realms/iceberg/protocol/openid-connect/token --user client1:s3cr3t -d 'grant_type=client_credentials' | jq -r .access_token) &&
Review Comment:
My assumption is that the token would just be "invalid".
It's also that the pipe "eliminates" the exit code from curl, but `jq` would fail then.
--
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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on code in PR #3612:
URL: https://github.com/apache/polaris/pull/3612#discussion_r2748020468
##########
getting-started/ceph/docker-compose.yml:
##########
@@ -148,10 +148,10 @@ services:
command:
- "-c"
- >-
- echo Creating Ceph bucket...;
- aws s3 mb s3://${S3_POLARIS_BUCKET};
- aws s3 ls;
- echo Bucket setup complete.;
+ echo Creating Ceph bucket... &&
+ aws s3 mb s3://${S3_POLARIS_BUCKET} &&
+ aws s3 ls &&
+ echo Bucket setup complete.
Review Comment:
So we have at most 2 setup services for these docker compose and you are adding `sleep 120` to pass the CI based on docker-compose (as exit of status code of zero will be treat as failed). Thus, this will also return status code of zero.
--
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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3612:
URL: https://github.com/apache/polaris/pull/3612#discussion_r2745621343
##########
getting-started/keycloak/docker-compose.yml:
##########
@@ -71,12 +71,12 @@ services:
command:
- "-c"
- >-
- apk add --no-cache jq &&
- chmod +x /polaris/create-catalog.sh &&
+ apk add --no-cache jq &&
token=$$(curl http://keycloak:8080/realms/iceberg/protocol/openid-connect/token --user client1:s3cr3t -d 'grant_type=client_credentials' | jq -r .access_token) &&
/polaris/create-catalog.sh realm-internal &&
/polaris/create-catalog.sh realm-external $$token &&
- /polaris/create-catalog.sh realm-mixed $$token
+ /polaris/create-catalog.sh realm-mixed $$token &&
+ sleep 120
Review Comment:
Probably best to let the polaris-setup services "tail forever" and have health-checks. Added that
--
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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on code in PR #3612:
URL: https://github.com/apache/polaris/pull/3612#discussion_r2748022955
##########
getting-started/keycloak/docker-compose.yml:
##########
@@ -71,12 +71,12 @@ services:
command:
- "-c"
- >-
- apk add --no-cache jq &&
- chmod +x /polaris/create-catalog.sh &&
+ apk add --no-cache jq &&
token=$$(curl http://keycloak:8080/realms/iceberg/protocol/openid-connect/token --user client1:s3cr3t -d 'grant_type=client_credentials' | jq -r .access_token) &&
/polaris/create-catalog.sh realm-internal &&
/polaris/create-catalog.sh realm-external $$token &&
- /polaris/create-catalog.sh realm-mixed $$token
+ /polaris/create-catalog.sh realm-mixed $$token &&
+ sleep 120
Review Comment:
Yes. That is what I ended up doing in my local prototyping with tail forever as status code of zero is not okay for compose service.
--
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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy merged PR #3612:
URL: https://github.com/apache/polaris/pull/3612
--
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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3612:
URL: https://github.com/apache/polaris/pull/3612#discussion_r2745411445
##########
getting-started/ceph/docker-compose.yml:
##########
@@ -148,10 +148,10 @@ services:
command:
- "-c"
- >-
- echo Creating Ceph bucket...;
- aws s3 mb s3://${S3_POLARIS_BUCKET};
- aws s3 ls;
- echo Bucket setup complete.;
+ echo Creating Ceph bucket... &&
+ aws s3 mb s3://${S3_POLARIS_BUCKET} &&
+ aws s3 ls &&
+ echo Bucket setup complete.
Review Comment:
WDYM?
--
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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3612:
URL: https://github.com/apache/polaris/pull/3612#discussion_r2749270997
##########
getting-started/ceph/docker-compose.yml:
##########
@@ -148,10 +148,10 @@ services:
command:
- "-c"
- >-
- echo Creating Ceph bucket...;
- aws s3 mb s3://${S3_POLARIS_BUCKET};
- aws s3 ls;
- echo Bucket setup complete.;
+ echo Creating Ceph bucket... &&
+ aws s3 mb s3://${S3_POLARIS_BUCKET} &&
+ aws s3 ls &&
+ echo Bucket setup complete.
Review Comment:
Ah, I think I know what you're referring to.
Those setup-bucket services behave in an "interesting" way, which is different than how the "final" polaris-setup services behave (in terms of exit behavior).
An "intermediate" setup-service like `setup-bucket`, on which for example `polaris` depends using the `service_completed_successfully` condition, is fine to exit and does not "break" a `docker compose up --detach --wait`.
A "final" setup-service like some `polaris-setup`, a service that no other service depends on, lets `docker compose up --detach --wait` fail when it exits.
The "crux" with those "final" setup-services is that the behavior isn't immediately obvious and depends on the timing of the relevant services and when exactly `docker compose up --detach --wait` "thinks" the whole compose thing is ready.
But this ^^ is all from experiments. It's tricky to find good documentation on this behavior. However, the behavior of `depends_on` (... a setup service) with `service_completed_successfully` condition 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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on code in PR #3612:
URL: https://github.com/apache/polaris/pull/3612#discussion_r2742954171
##########
getting-started/ceph/docker-compose.yml:
##########
@@ -148,10 +148,10 @@ services:
command:
- "-c"
- >-
- echo Creating Ceph bucket...;
- aws s3 mb s3://${S3_POLARIS_BUCKET};
- aws s3 ls;
- echo Bucket setup complete.;
+ echo Creating Ceph bucket... &&
+ aws s3 mb s3://${S3_POLARIS_BUCKET} &&
+ aws s3 ls &&
+ echo Bucket setup complete.
Review Comment:
This will return 0 as well no?
##########
getting-started/keycloak/docker-compose.yml:
##########
@@ -71,12 +71,12 @@ services:
command:
- "-c"
- >-
- apk add --no-cache jq &&
- chmod +x /polaris/create-catalog.sh &&
+ apk add --no-cache jq &&
token=$$(curl http://keycloak:8080/realms/iceberg/protocol/openid-connect/token --user client1:s3cr3t -d 'grant_type=client_credentials' | jq -r .access_token) &&
/polaris/create-catalog.sh realm-internal &&
/polaris/create-catalog.sh realm-external $$token &&
- /polaris/create-catalog.sh realm-mixed $$token
+ /polaris/create-catalog.sh realm-mixed $$token &&
+ sleep 120
Review Comment:
So yeah, while I was doing a local prototyping, I ran into this issue as well where `--exit-code-from` is not happy with this. What I ended up doing is to use `tail /dev/null` to keep the service up then use health check on service ready instead of completed. Sleep 120 here will work but if for whatever reason these bash scripts took more than 120s (assuming they get very complex later on), this will bite us.
--
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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on code in PR #3612:
URL: https://github.com/apache/polaris/pull/3612#discussion_r2742931210
##########
getting-started/keycloak/docker-compose.yml:
##########
@@ -71,12 +71,12 @@ services:
command:
- "-c"
- >-
- apk add --no-cache jq &&
- chmod +x /polaris/create-catalog.sh &&
+ apk add --no-cache jq &&
token=$$(curl http://keycloak:8080/realms/iceberg/protocol/openid-connect/token --user client1:s3cr3t -d 'grant_type=client_credentials' | jq -r .access_token) &&
Review Comment:
should we do implicitly `--fail-with-body` here too for curl?
--
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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "MonkeyCanCode (via GitHub)" <gi...@apache.org>.
MonkeyCanCode commented on code in PR #3612:
URL: https://github.com/apache/polaris/pull/3612#discussion_r2748587530
##########
getting-started/keycloak/docker-compose.yml:
##########
@@ -71,12 +71,12 @@ services:
command:
- "-c"
- >-
- apk add --no-cache jq &&
- chmod +x /polaris/create-catalog.sh &&
+ apk add --no-cache jq &&
token=$$(curl http://keycloak:8080/realms/iceberg/protocol/openid-connect/token --user client1:s3cr3t -d 'grant_type=client_credentials' | jq -r .access_token) &&
Review Comment:
That is fair enough. Yeah. Just thought to be consistent. But not a blocker.
--
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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3612:
URL: https://github.com/apache/polaris/pull/3612#discussion_r2745410254
##########
getting-started/keycloak/docker-compose.yml:
##########
@@ -71,12 +71,12 @@ services:
command:
- "-c"
- >-
- apk add --no-cache jq &&
- chmod +x /polaris/create-catalog.sh &&
+ apk add --no-cache jq &&
token=$$(curl http://keycloak:8080/realms/iceberg/protocol/openid-connect/token --user client1:s3cr3t -d 'grant_type=client_credentials' | jq -r .access_token) &&
/polaris/create-catalog.sh realm-internal &&
/polaris/create-catalog.sh realm-external $$token &&
- /polaris/create-catalog.sh realm-mixed $$token
+ /polaris/create-catalog.sh realm-mixed $$token &&
+ sleep 120
Review Comment:
Man, docker-compose isn't a good friend at all :(
--
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] Guides: fix setup scripts to yield correct exit code [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #3612:
URL: https://github.com/apache/polaris/pull/3612#discussion_r2745429284
##########
getting-started/keycloak/docker-compose.yml:
##########
@@ -71,12 +71,12 @@ services:
command:
- "-c"
- >-
- apk add --no-cache jq &&
- chmod +x /polaris/create-catalog.sh &&
+ apk add --no-cache jq &&
token=$$(curl http://keycloak:8080/realms/iceberg/protocol/openid-connect/token --user client1:s3cr3t -d 'grant_type=client_credentials' | jq -r .access_token) &&
/polaris/create-catalog.sh realm-internal &&
/polaris/create-catalog.sh realm-external $$token &&
- /polaris/create-catalog.sh realm-mixed $$token
+ /polaris/create-catalog.sh realm-mixed $$token &&
+ sleep 120
Review Comment:
Neither `tail /dev/null` nor `sleep 120` is particularly great.
I wanted to stay away from the added complexity of health-checks for the setup-scripts though.
Let me think a bit about this.
--
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