Skip to content
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

[native] Update dependency dockerfile #24257

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

majetideepak
Copy link
Collaborator

Description

  • Remove unnecessary modules to save space.
  • Install newer dependencies.

Motivation and Context

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* ... :pr:`12345`
* ... :pr:`12345`

Hive Connector Changes
* ... :pr:`12345`
* ... :pr:`12345`

If release note is NOT required, use:

== NO RELEASE NOTE ==

@majetideepak majetideepak requested a review from a team as a code owner December 13, 2024 14:05
@prestodb-ci prestodb-ci added the from:IBM PR from IBM label Dec 13, 2024
@prestodb-ci prestodb-ci requested review from a team, bibith4 and namya28 and removed request for a team December 13, 2024 14:05
@majetideepak majetideepak requested review from czentgr and removed request for bibith4 and namya28 December 13, 2024 14:05
@@ -0,0 +1,10 @@
../scripts/setup-$1.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

We need the license header in this new file.

../velox/scripts/setup-adapters.sh && \
../scripts/setup-adapters.sh ) && \
rm -rf build
RUN chmod +x /scripts/install_minimal_dependencies.sh && /scripts/install_minimal_dependencies.sh centos
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the 644 for the script on purpose? Why don't we give it execute (755) when it is added in the first place and don't need the chmod?

The name of the script - minimal_dependencies sounds like the minimum number of dependencies to build when in fact we install everything. Minimal refers to the size of the resulting image. Trying to think of a better name because this might cause confusion when looking only at the name.

Maybe something with install_and_reduce_size.sh or something similar?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
from:IBM PR from IBM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants