Skip to content

Unify the Docker image build scripts#12507

Merged
martint merged 2 commits intotrinodb:masterfrom
nineinchnick:simplify-building-docker-image
May 27, 2022
Merged

Unify the Docker image build scripts#12507
martint merged 2 commits intotrinodb:masterfrom
nineinchnick:simplify-building-docker-image

Conversation

@nineinchnick
Copy link
Copy Markdown
Member

Description

Unify both build scripts into one and make it recognize flags and print a helpful usage message. Use Maven to download artifacts when building an image for a released version.

Is this change a fix, improvement, new feature, refactoring, or other?
refactor of build utility scripts

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)
build scripts

How would you describe this change to a non-technical end user or system administrator?
n/a

Related issues, pull requests, and links

Documentation

( ) No documentation is needed.
(x) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@nineinchnick nineinchnick force-pushed the simplify-building-docker-image branch from c76b785 to b7ef3ff Compare May 23, 2022 13:53
@nineinchnick nineinchnick requested a review from findepi May 23, 2022 13:53
@nineinchnick nineinchnick force-pushed the simplify-building-docker-image branch 2 times, most recently from de50f40 to ffd367f Compare May 24, 2022 08:29
Copy link
Copy Markdown
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

skimmed

@hashhar ptal

@nineinchnick nineinchnick force-pushed the simplify-building-docker-image branch from ffd367f to 8e1d9fc Compare May 24, 2022 11:07
@hashhar
Copy link
Copy Markdown
Member

hashhar commented May 25, 2022

CI hit #12535

@hashhar
Copy link
Copy Markdown
Member

hashhar commented May 25, 2022

cc: @martint FYI - not much changes for the releases.

./build.sh -r TRINO_VERSION

Copy link
Copy Markdown
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

LGTM but let's wait for @martint to acknowledge.

@martint
Copy link
Copy Markdown
Member

martint commented May 25, 2022

Thanks, this is a nice improvement! In particular, it saves having to re-download the tgz artifacts from maven central if something fails when building the docker image.

@martint
Copy link
Copy Markdown
Member

martint commented May 25, 2022

Before merging, can you make the corresponding change to https://github.com/trinodb/release-scripts/blob/master/trino-docker.sh so we can get them both in at the same time?

@nineinchnick
Copy link
Copy Markdown
Member Author

@martint here you go: trinodb/release-scripts#4

@martint martint merged commit 85650c3 into trinodb:master May 27, 2022
@github-actions github-actions bot added this to the 383 milestone May 27, 2022
@nineinchnick nineinchnick deleted the simplify-building-docker-image branch May 27, 2022 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants