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.

Sign up for GitHub

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

Jump to bottom

Use the Airbyte Base Java Image for all services #17843

Merged
merged 8 commits into from
Oct 12, 2022
Merged

Use the Airbyte Base Java Image for all services #17843

merged 8 commits into from
Oct 12, 2022

Conversation

jdpgrailsdev
Copy link
Contributor

@jdpgrailsdev jdpgrailsdev commented Oct 11, 2022 •
edited
Loading

What

  • Enable the option to use APM tracing in all services

How

  • Use airbyte/airbyte-base-java-image for all services
  • Remove the use of JDK_VERSION build argument in service builds to avoid conflicts between image and version tags.

Recommended reading order

  1. build.gradle
  2. Dockerfile

@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets October 11, 2022 14:45 Inactive
@github-actions github-actions bot added area/platform issues related to the platform area/server area/worker Related to worker labels Oct 11, 2022
@jdpgrailsdev
Copy link
Contributor Author

@benmoriceau This will not have the same issue that you ran into on Friday. I didn't realize that the JDK_VERSION build argument exists for connector builds, not platform builds. I have verified that the JDK_IMAGE build argument is only used by platform image builds, so setting that to the airbyte/java-datadog-tracer-base image will not impact connectors.

It is also worth noting that it is not really possible at the moment for the JDK_IMAGE build argument to be set to anything but an Amazon Corretto based base image, as there are commands in the Dockerfiles that use specific tools provided by the Amazon-based images. This pre-dates this PR and is something I am currently discussing with @git-phu and @davinchia to see how we should handle this in OSS.

@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets October 11, 2022 15:21 Inactive
benmoriceau
benmoriceau approved these changes Oct 11, 2022
View reviewed changes
Copy link
Contributor

@benmoriceau benmoriceau left a comment

Choose a reason for hiding this comment

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

LGTM, maybe we should change the title of the PR. Otherwise it looks good

@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets October 11, 2022 17:07 Inactive
@jdpgrailsdev jdpgrailsdev changed the title Use the Amazon Corretto based base image for all services Use the Airbyte Base Java Image for all services Oct 11, 2022
@jdpgrailsdev
Copy link
Contributor Author

@benmoriceau Sorry to make you re-review: PR is now updated to use the new airbyte/airbyte-base-java-image.

@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets October 11, 2022 17:11 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets October 11, 2022 17:58 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets October 11, 2022 18:17 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets October 11, 2022 18:58 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets October 11, 2022 19:13 Inactive
@jdpgrailsdev jdpgrailsdev temporarily deployed to more-secrets October 11, 2022 21:21 Inactive
@jdpgrailsdev jdpgrailsdev merged commit 246172b into master Oct 12, 2022
@jdpgrailsdev jdpgrailsdev deleted the jonathan/apm-docker-image branch October 12, 2022 12:59
This was referenced Oct 13, 2022
Closed
Merged
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
@jdpgrailsdev @jhammarstedt
Use the Airbyte Base Java Image for all services (airbytehq#17843)
46b6cf3 
* Use the Amazon Corretto based base image for all services

* Add comment

* Use airbyte-base-java-image
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benmoriceau benmoriceau benmoriceau approved these changes

@colesnodgrass colesnodgrass Awaiting requested review from colesnodgrass

@gosusnp gosusnp Awaiting requested review from gosusnp

@alovew alovew Awaiting requested review from alovew

Assignees
No one assigned
Labels
area/platform issues related to the platform area/server area/worker Related to worker
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jdpgrailsdev @benmoriceau