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

yum clean all after every yum install to save space #27555

Merged
merged 7 commits into from Jun 23, 2023
Merged

Conversation

evantahler
Copy link
Contributor

@evantahler evantahler commented Jun 21, 2023

This PR expands on #27543, closes #27543

After every time we yum install a package, we should also yum clean all so that we reclaim any space in the docker image yum used to cache packages, headers, and such. When possible, this should be done in the same RUN command, so there isn't a big image layer that needs to be cached and then removed

If the connector tests build, then I think this PR should be OK. We should publish new versions of these connectors with the smaller images

@github-actions
Copy link
Contributor

github-actions bot commented Jun 21, 2023

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

  • PR name follows PR naming conventions
  • Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan and you've followed all steps in the Breaking Changes Checklist
  • Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • Secrets in the connector's spec are annotated with airbyte_secret
  • All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
  • Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
  • The connector tests are passing in CI
  • You've updated the connector's metadata.yaml file (new!)
  • If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

  1. Check for hidden checklists in your PR description

  2. Toggle the github label checklist-action-run on/off to re-run the checklist CI.

@alafanechere
Copy link
Contributor

alafanechere commented Jun 21, 2023

@evantahler these changes will have no effect on java connectors. They need to happen on the dagger code here

@github-actions
Copy link
Contributor

github-actions bot commented Jun 21, 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 (13)

Connector Version Changelog Publish
destination-bigquery 1.4.5
(diff seed version)
destination-bigquery-denormalized 1.4.1
destination-clickhouse 0.2.5
(diff seed version)
destination-clickhouse-strict-encrypt 0.2.5 🔵
(ignored)
🔵
(ignored)
destination-databricks 1.1.0
destination-mssql 0.1.25
(diff seed version)
destination-mssql-strict-encrypt 0.1.25 🔵
(ignored)
🔵
(ignored)
destination-redshift 0.4.9
(diff seed version)
destination-s3 0.4.2
(diff seed version)
destination-s3-glue 0.1.7
destination-snowflake 1.0.6
(diff seed version)
destination-starburst-galaxy 0.0.1
destination-tidb 0.1.4
(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
Copy link
Contributor Author

Umm... why is the formatter messing with gradle code and removing things? 33f4c92

@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Jun 21, 2023
@evantahler
Copy link
Contributor Author

The formatter is doing crazy things. 3074020 is the last commit I intended

Copy link
Contributor Author

@evantahler evantahler left a comment

Choose a reason for hiding this comment

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

I think this PR is ready for review.
@alafanechere - I'm surprised that none of the connector tests ran, given that I changed their metadata files. That said, it looks like the base build did at least build all the effected java connectors
Now that the PR is ready-for-review, the tests should run :D

@evantahler evantahler marked this pull request as ready for review June 22, 2023 17:39
@evantahler evantahler requested a review from a team as a code owner June 22, 2023 17:39
@@ -655,6 +655,7 @@ def with_integration_base_java_and_normalization(context: PipelineContext, build
return (
with_integration_base_java(context, build_platform)
.with_exec(["yum", "install", "-y"] + yum_packages_to_install)
.with_exec(["yum", "clean", "all"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it cool to make a single line of code change that impacts all java connectors 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

VERY COOL

@evantahler
Copy link
Contributor Author

Screenshot 2023-06-22 at 4 50 12 PM 👀 hopefully I'll get some test reports soon...

@octavia-squidington-iii
Copy link
Collaborator

destination-redshift test report (commit e69bc8d2d6) - ✅

⏲️ Total pipeline duration: 2014 seconds

Step Result
Validate airbyte-integrations/connectors/destination-redshift/metadata.yaml
Connector version semver check.
Connector version increment check.
QA checks
Build connector tar
Build destination-redshift docker image for platform linux/x86_64
Unit tests
Build airbyte/normalization-redshift:dev
Integration tests

🔗 View the logs here

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=destination-redshift test

@octavia-squidington-iii
Copy link
Collaborator

destination-s3 test report (commit e69bc8d2d6) - ✅

⏲️ Total pipeline duration: 782 seconds

Step Result
Validate airbyte-integrations/connectors/destination-s3/metadata.yaml
Connector version semver check.
Connector version increment check.
QA checks
Build connector tar
Build destination-s3 docker image for platform linux/x86_64
Unit tests
Integration tests

🔗 View the logs here

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=destination-s3 test

@octavia-squidington-iii
Copy link
Collaborator

destination-mssql-strict-encrypt test report (commit e69bc8d2d6) - ❌

⏲️ Total pipeline duration: 1713 seconds

Step Result
Validate airbyte-integrations/connectors/destination-mssql-strict-encrypt/metadata.yaml
Connector version semver check.
Connector version increment check.
QA checks
Build connector tar
Build destination-mssql-strict-encrypt docker image for platform linux/x86_64
Unit tests
Build airbyte/normalization-mssql:dev
Integration tests

🔗 View the logs here

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=destination-mssql-strict-encrypt test

@alafanechere
Copy link
Contributor

@evantahler the destination-mssql tests are taking forever. Currently stuck at running its integration test:

MSSQLDestinationAcceptanceTest > testEntrypointEnvVar() STARTED

https://alpha.dagger.cloud/runs/28d8e3da-21ea-47d5-b077-796452fc0157

@evantahler
Copy link
Contributor Author

@evantahler the destination-mssql tests are taking forever. Currently stuck at running its integration test:

Wild. Well... nothing that was added in this PR would have changed that... either the docker image would build... or not. I'm getting enough signal that the images are building OK, so I'm going to merge this PR

@evantahler evantahler merged commit 1f6aef9 into master Jun 23, 2023
22 of 31 checks passed
@evantahler evantahler deleted the evan/yum-clean branch June 23, 2023 20:53
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