-
Notifications
You must be signed in to change notification settings - Fork 457
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
chore(deps): relax jsii dependencies #1288
Conversation
Looked a bit into aws/jsii#3078 and yes, it's something which we should somehow address upstream. I see two options, both around the way module imports are generated in jsii.
There's no clean way to just configure this on our end, so we'll work on this upstream. note: it's only an issue for user generated code like in the example. When packaged as package it's working similarily to the |
The goal of this PR is, to find a way for generated Python code to work with namespaced modules regardless of the way it is consumed. While the current implementation of submodule imports seem to work fine when distributed as packages, it's breaking apart when the generated code is imported relatively as part of a single code base. That's what we do in cdktf and the reason why we have locked jsii to `1.37.0` at the moment. The general idea is, to generate relative imports rather than fqn imports in Python. This could be a potential way to address #3078 ```python # Before import jsii_calc.submodule import jsii_calc.submodule.nested_submodule import jsii_calc.submodule.nested_submodule.deeply_nested # After from . import submodule from .submodule import nested_submodule from .submodule.nested_submodule import deeply_nested ``` Besides from a few snapshots being changed, it seems like the tests are still working with this change. However, I'm not sure about the general implications. So, this entire PR is really meant to be a conversation starter. I have confirmed that an import like `import . from some_aws_namespace` would work for us in cdktf. ## References - #3078 - #3049 - hashicorp/terraform-cdk#1152 - hashicorp/terraform-cdk#1288 --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
The upstream fix in aws/jsii#3181 got merged and is likely gonna be released early next week |
This one was superseded by #1336, wasn't it? |
I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
With the 0.7 release we fixed jsii to
1.37.0
because of #1152 and aws/jsii#3078This pull request aims to relax that restriction again. I'm not exactly sure if our changes to the namespacing fixed the issues already or if we have to push for aws/jsii#3078 - Tests will tell :)