[CDK] Fix ValueError in Python CDK

[maver1ck]

Fixes #33795

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 7, 2024 6:04pm

[CLAassistant]

All committers have signed the CLA.

[marcosmarxm]

Can you add a unit test to validate the change?

[maver1ck]

Hi @marcosmarxm
Could you please point me to similar tests that can be inspiration ?

[girarda]

@maver1ck you can add tests to the test files for the classes you modified. Here's the test file for the token provider as an example.

I'm a little curious about the root cause or how to reproduce the issue. Do you know if this is a problem specific to python 3.11?

[marcosmarxm]

@maver1ck did you have time to take a look in Alex's comments?

[maver1ck]

Yep. I will look into it next week.

[andrii-porokhnavets]

Hey @maver1ck, thanks for fixing this issue! I face the same and it would be really cool to merge this PR. Could you say when you are going to finish it?

[marcosmarxm]

@maver1ck were you able to take a look and provide some unit tests?

[aaronsteers]

@maver1ck - Thanks for creating this PR. I can confirm some PyAirbyte users have started to run into this issue under Python 3.11. The workaround is to downgrade to 3.10, if possible, but I'll add my voice to those hoping we can incorporate+merge this. 🙂

[natikgadzhi]

I just hit the same problem in source-declarative-manifest. Let me take a look, see if I can add tests and merge this.

[natikgadzhi]

I think that a meaningful way to validate this would be:

1. To make sure that we run Python CDK tests on Python 3.9, 3.10, and 3.11. Adding 3.11 is important because it's exactly what we're trying to fix here, there's no error on 3.10.
2. Make sure that existing code without the PR fails on 3.11 tests, but does work correctly with the PR in.

I don't know if new test cases will be needed, since the failure is when you're trying to even import the CDK. What we want to validate is that you can still instantiate all those classes and their behavior does not change, but that's in currently existing tests mostly.

[natikgadzhi]

@alafanechere this aged like milk, probably not worth rebasing, but the actual issue of not being compatible with 3.11 is there. We should clean this out.

[natikgadzhi]

This just shipped in #38846!