-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
base-images: release a base image for our java connectors #49831
base-images: release a base image for our java connectors #49831
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
709f1b1
to
10dc398
Compare
RUN yum update -y --security | ||
RUN yum install -y tar openssl findutils |
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.
I know this is just the example, but looking into the code further down in this PR, it looks like we're following what this example does.
We want to limit the number of RUN
calls in our image creation process. Each RUN
will create a new layer and further increase the size of our base image.
You can see on the platform side how we've collapsed multiple RUN
calls into a single call to reduce our base image size.
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.
@colesnodgrass I can definitely bundle these three commands into one. Thank you for the suggestion.
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.
Thank you for the tips @colesnodgrass . 50mb saved 👍
10dc398
to
6bd024b
Compare
|
||
|
||
class AirbyteJavaConnectorBaseImage(bases.AirbyteConnectorBaseImage): | ||
# TODO: remove this once we want to build the base image with the airbyte user. |
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 I expect mimics the current setup?
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.
Yes the current user is root. The parent class sets it to airbyte, so I override the USER
with root
to keep v1 expected user be root
.
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.
Approving since blast radius is zero. <3
6bd024b
to
569f2df
Compare
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.
LGTM as far as I can tell
repository: Final[str] = "airbyte/java-connector-base" | ||
|
||
DD_AGENT_JAR_URL: Final[str] = "https://dtdg.co/latest-java-tracer" | ||
BASE_SCRIPT_URL = "https://raw.githubusercontent.com/airbytehq/airbyte/6d8a3a2bc4f4ca79f10164447a90fdce5c9ad6f9/airbyte-integrations/bases/base/base.sh" |
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.
consider lifting the git commit sha to a dedicated constant
What
Relates to https://github.com/airbytehq/airbyte-internal-issues/issues/11151
Declare a base image for our java connectors by copying the build logic currently declared in
airbyte-ci
herePrelease it to DockerHub:
airbyte/java-connector-base:1.0.0-rc.1