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

Multi-architecture normalization build (local) #26677

Merged
merged 2 commits into from
May 26, 2023
Merged

Conversation

evantahler
Copy link
Contributor

When building and testing normalization locally, we need to force the base images to match the local host OS.

This is not a problem when publishing the connectors as airbyte-ci/dagger handles this for us

When building and testing normalization locally, we need to force the base images to match the local host OS.

This is not a problem when publishing the connectors as `airbyte-ci`/dagger handles this for us
@github-actions
Copy link
Contributor

github-actions bot commented May 26, 2023

Affected Connector Report

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to do the following as needed:

  • Run integration tests
  • Bump connector or module version
  • Add changelog
  • Publish the new version

✅ Sources (0)

Connector Version Changelog Publish
  • See "Actionable Items" below for how to resolve warnings and errors.

❌ Destinations (15)

Connector Version Changelog Publish
destination-bigquery 1.4.4
destination-bigquery-denormalized 1.4.1
destination-clickhouse 0.2.3
destination-clickhouse-strict-encrypt 0.2.3 🔵
(ignored)
🔵
(ignored)
destination-mssql 0.1.23
destination-mssql-strict-encrypt 0.1.23 🔵
(ignored)
🔵
(ignored)
destination-mysql 0.1.20
destination-mysql-strict-encrypt 0.1.21
(mismatch: 0.1.20)
🔵
(ignored)
🔵
(ignored)
destination-oracle 0.1.19
destination-oracle-strict-encrypt 0.1.19 🔵
(ignored)
🔵
(ignored)
destination-postgres 0.3.27
destination-postgres-strict-encrypt 0.3.27 🔵
(ignored)
🔵
(ignored)
destination-redshift 0.4.8
destination-snowflake 1.0.4
destination-tidb 0.1.1
(diff seed version)
  • See "Actionable Items" below for how to resolve warnings and errors.

👀 Other Modules (1)

  • base-normalization

Actionable Items

(click to expand)

Category Status Actionable Item
Version
mismatch
The version of the connector is different from its normal variant. Please bump the version of the connector.

doc not found
The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like source-jdbc is not published or documented). Please double-check to make sure that it is not a bug.
Changelog
doc not found
The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like source-jdbc is not published or documented). Please double-check to make sure that it is not a bug.

changelog missing
There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog.
Publish
not in seed
The connector is not in the cloud or oss registry, so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that you have added a metadata.yaml file and the expected registries are enabled.

@evantahler evantahler requested a review from edgao May 26, 2023 17:14
@evantahler evantahler marked this pull request as ready for review May 26, 2023 17:14
@evantahler evantahler requested a review from a team as a code owner May 26, 2023 17:14
@evantahler
Copy link
Contributor Author

There may also be a slight speed improvement on building these images now that they aren't emulating some other arch too...

Copy link
Contributor

@edgao edgao left a comment

Choose a reason for hiding this comment

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

nice. this has no impact on prod, right? because /publish doesn't go through this path anymore

@evantahler
Copy link
Contributor Author

nice. this has no impact on prod, right? because /publish doesn't go through this path anymore

I think this should be OK for 2 reasons:

@evantahler evantahler merged commit 75240b0 into master May 26, 2023
20 checks passed
@evantahler evantahler deleted the evantahler-patch-1 branch May 26, 2023 17:57
cgardens added a commit that referenced this pull request May 31, 2023
cgardens added a commit that referenced this pull request May 31, 2023
cgardens added a commit that referenced this pull request May 31, 2023
cgardens added a commit that referenced this pull request May 31, 2023
evantahler added a commit that referenced this pull request May 31, 2023
marcosmarxm pushed a commit to natalia-miinto/airbyte that referenced this pull request Jun 8, 2023
* Multi-architecture normalization build (local)

When building and testing normalization locally, we need to force the base images to match the local host OS.

This is not a problem when publishing the connectors as `airbyte-ci`/dagger handles this for us

* Update build.gradle
@davinchia
Copy link
Contributor

davinchia commented Jun 9, 2023

@evantahler was this not detecting the correct local arch before this change?

I ran into this because I was attempting to build an amd image on my arm machine. I was doing so by setting DOCKER_BUILD_PLATFORM from here. However this was failing as the normalization builds only build arm images from this change onwards.

@evantahler
Copy link
Contributor Author

no... because the base image we use is so old, they only ever published AMD images... so any image built on it remains AMD unless you do something drastic... like this! Anyway, all of this is going away soon so 🤷

@evantahler
Copy link
Contributor Author

You can update the changes if you want to look at the ENV DOCKER_BUILD_PLATFORM first, and if that's missing, use the local arch, that would be fine

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

Successfully merging this pull request may close these issues.

None yet

4 participants