Skip to content
This repository has been archived by the owner on Sep 14, 2020. It is now read-only.

[246] Prepare for aiohttp v4, but stay with v3 #277

Merged
merged 2 commits into from
Jan 9, 2020

Conversation

nolar
Copy link
Contributor

@nolar nolar commented Dec 23, 2019

aiohttp v4 would bring few changes incompatible with Kopf and would break both the CI/CD pipelines and the Kopf-based operators. Some of these issues can be proactively fixed now. Others can be prevented by non-upgrading to v4.

Issue : closes #246

Description

The main incompatibility is that it is now impossible to inherit from aiohttp.ClientSession — explicitly restrictred in aiohttp.

This PR converts our APISession from inheriting aiohttp.ClientSession to containing aiohttp.ClientSession (i.e. a composition instead of inheritance).

Since it is not a session anymore, it is also renamed to a context — the closest semantic equivalent I could find. Nevertheless, it is not part of the public interface, so we are free to change and rename it as needed.

Beside of that, it also resolves the design flaw with few private/protected fields injected into the conceptually irrelevant 3rd-party session objects.


This PR, however, does NOT make Kopf fully compatible with aiohttp v4. Beside of the ClientSession, aresponses (used in tests) has some issues with the new aiohttp, and aiohttp v4 itself has incompatibilities with async-timeout v4.

It is not wise to dive that deep into the dependencies. Instead, we restrict aiohttp to major v3, and wait until v4 is released and stabilised (now, it is all in alpha).

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • Refactor/improvements

@nolar nolar added the bug Something isn't working label Dec 23, 2019
@nolar nolar requested a review from samurang87 as a code owner December 23, 2019 12:50
@zincr
Copy link

zincr bot commented Dec 23, 2019

🤖 zincr found 0 problems , 0 warnings

✅ Large Commits
✅ Approvals
✅ Specification
✅ Dependency Licensing

@zincr
Copy link

zincr bot commented Dec 23, 2019

🤖 zincr found 1 problem , 0 warnings

❌ Approvals
✅ Large Commits
✅ Specification
✅ Dependency Licensing

Details on how to resolve are provided below


Approvals

All proposed changes must be reviewed by project maintainers before they can be merged

Not enough people have approved this pull request - please ensure that 1 additional user, who have not contributed to this pull request approve the changes.

  • ✅ Approved by PR author @nolar
  • ❌ 1 additional approval needed
     

@nolar nolar merged commit 848e1da into zalando-incubator:master Jan 9, 2020
@nolar nolar deleted the 246-aiohttp4-readiness branch January 9, 2020 11:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kopf is incompatible with aiohttp 4.x
2 participants