-
Notifications
You must be signed in to change notification settings - Fork 178
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.
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
Update CI server docker container #464
Conversation
docker images | ||
docker run -d -p 5433:5433 jbfavre/vertica | ||
sleep 60 | ||
docker pull vertica/vertica-ce:12.0.0-0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks much more maintainable. I think it might be worth putting a comment in explaining that -d runs the container in the background. I'm also curious about the two ports. Why -p for 5433 and 5444?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are following the example in the vetica-ce dockerhub page: https://hub.docker.com/r/vertica/vertica-ce
which reserve both 5433 and 5444
.github/workflows/ci.yaml
Outdated
--name vertica_docker \ | ||
vertica/vertica-ce | ||
echo "Vertica startup ..." | ||
until docker exec vertica_docker /opt/vertica/bin/vsql -c "SELECT 1" > /dev/null 2>&1; do \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is much better than waiting 60 seconds! I like it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me. just some minor things. Have u done the sql-go client CI update? If not I can do it.
docker images | ||
docker run -d -p 5433:5433 jbfavre/vertica | ||
sleep 60 | ||
docker pull vertica/vertica-ce:12.0.0-0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are following the example in the vetica-ce dockerhub page: https://hub.docker.com/r/vertica/vertica-ce
which reserve both 5433 and 5444
.github/workflows/ci.yaml
Outdated
@@ -21,12 +21,17 @@ jobs: | |||
env: | |||
VERTICA_CE_URL: "https://vertica-community-edition-for-testing.s3.amazonaws.com/XCz9cp7m/vertica-12.0.0-0.x86_64.RHEL6.rpm" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this still needed anymore?
Migrate to use official Vertica CE docker container.