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

Bump aiohttp across services, images, and packages #3178

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

SorenSpicknall
Copy link
Contributor

Description

Bumps aiohttp across each relevant import, except in /airflow (which we try to keep in relative parity with the version deployed via Composer). Doesn't touch the cryptography import mentioned in #3165 for the same reason (parity for Composer in /airflow).

Resolves #3165

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

How has this been tested?

Each relevant image built locally prior to merging. Some CI tasks will also build images.

Post-merge follow-ups

  • No action required
  • Actions required (specified below)

Copy link

Copy link
Contributor

@lauriemerrell lauriemerrell left a comment

Choose a reason for hiding this comment

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

Technical changes LGTM, but nitpick that airflow is checked off for aiohttp in #3165 when not changed in this PR.

For actions after merge, what would you plan to do with the two Airflow changes that aren't going to be made now -- would you just close those two dependabot PRs with a comment, or leave them open with a comment, or maybe make a new separate issue to start aggregating changes that we assume should be handled by the upcoming Airflow upgrade?

@SorenSpicknall
Copy link
Contributor Author

Technical changes LGTM, but nitpick that airflow is checked off for aiohttp in #3165 when not changed in this PR.

For actions after merge, what would you plan to do with the two Airflow changes that aren't going to be made now -- would you just close those two dependabot PRs with a comment, or leave them open with a comment, or maybe make a new separate issue to start aggregating changes that we assume should be handled by the upcoming Airflow upgrade?

Before I took on primary stewardship of this genre of task, it appears that previous maintainers tended to close accompanying Issues. I'm supportive of that general tact, since upgrades to Airflow itself naturally prompt upgrades to underlying dependencies and it's not particularly useful to track these small dependency changes in detail.

One thing I will do is create an Issue to configure dependabot behavior to ignore certain parts of the repo, something we don't do by default - that way we're not prompted for updates to certain files that need to be manually maintained, like Dockerfile.local in the /airflow folder.

@SorenSpicknall SorenSpicknall merged commit 2de214d into main Dec 11, 2023
17 checks passed
@SorenSpicknall SorenSpicknall deleted the soren-aiohttp_update branch December 11, 2023 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deal with aiohttp and cryptography Dependabot PRs (December 2023)
2 participants