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

Python aws-cdk-lib: Invalid escape characters #4532

Closed
DFEvans opened this issue Mar 22, 2023 · 12 comments · Fixed by #4538
Closed

Python aws-cdk-lib: Invalid escape characters #4532

DFEvans opened this issue Mar 22, 2023 · 12 comments · Fixed by #4538
Labels
bug This issue is a bug. documentation This is a problem with documentation. effort/small Small work item – less than a day of effort jsii p2

Comments

@DFEvans
Copy link

DFEvans commented Mar 22, 2023

Describe the bug

In the Python bindings, the docstring on aws_cdk.aws_secretsmanager.__init__ contains the text:

[...] If none are defined then the following set is used: `% +~`#$&*()|[]{}:;<>?!'/@"\`.

At the end of this line, the backslash-backtick sequence is interpreted by Python as an invalid escape character.

Expected Behavior

The code does not contain invalid escape sequences.

Current Behavior

The code contains an invalid escape sequence, which causes a warning. This is raising a SyntaxError for a specific Python install we have (3.10.9, potentially a vendorised version - proving difficult to track down exactly why only one install is causing trouble!).

Reproduction Steps

Import CDK in Python - check for warnings or errors related to escape sequences.

Possible Solution

It appears the docstring is taken from the Markdown at:

https://github.com/aws/aws-cdk/blob/52edf299a42f17254da0151bb939bcb303d0e5e6/packages/%40aws-cdk/aws-secretsmanager/README.md

In Python alone, replacing \ with \\ would be the fix, but a multi-language solution is presumably more complex!

Additional Information/Context

No response

CDK CLI Version

2.68.0 (build 25fda51)

Framework Version

No response

Node.js Version

v18.9.0

OS

Ubuntu

Language

Python

Language Version

No response

Other information

No response

@DFEvans DFEvans added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 22, 2023
@pahud
Copy link
Contributor

pahud commented Mar 22, 2023

Thank you for your report. Can you provide some code snippets or repository that we can repro in our account?

@pahud pahud added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Mar 22, 2023
@DFEvans
Copy link
Author

DFEvans commented Mar 22, 2023

With the troublesome Python install, it's as simple as an import aws_cdk as cdk.

The traceback we're seeing (from a pytest runner, hence the import indirection) is:

__________________ ERROR collecting deploy/tests/test_app.py ___________________
.venv/lib/python3.10/site-packages/_pytest/python.py:618: in _importtestmodule
    mod = import_path(self.path, mode=importmode, root=self.config.rootpath)
.venv/lib/python3.10/site-packages/_pytest/pathlib.py:533: in import_path
    importlib.import_module(module_name)
/opt/hostedtoolcache/Python/3.10.9/x64/lib/python3.10/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
<frozen importlib._bootstrap>:1050: in _gcd_import
    ???
<frozen importlib._bootstrap>:1027: in _find_and_load
    ???
<frozen importlib._bootstrap>:1006: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:688: in _load_unlocked
    ???
.venv/lib/python3.10/site-packages/_pytest/assertion/rewrite.py:168: in exec_module
    exec(co, module.__dict__)
deploy/tests/test_app.py:1: in <module>
    import aws_cdk as cdk
.venv/lib/python3.10/site-packages/aws_cdk/__init__.py:[28](https://github.com/SatelliteVu/image-stacking-poc/actions/runs/4491765601/jobs/7900691515#step:9:29)[46](https://github.com/SatelliteVu/image-stacking-poc/actions/runs/4491765601/jobs/7900691515#step:9:47)2: in <module>
    from . import aws_backup
.venv/lib/python3.10/site-packages/aws_cdk/aws_backup/__init__.py:268: in <module>
    from ..aws_rds import (
.venv/lib/python3.10/site-packages/aws_cdk/aws_rds/__init__.py:1024: in <module>
    from ..aws_secretsmanager import (
E     File "/home/runner/work/image-stacking-poc/image-stacking-poc/.venv/lib/python3.10/site-packages/aws_cdk/aws_secretsmanager/__init__.py", line 1
E       '''
E       ^^^
E   SyntaxError: invalid escape sequence '\`'

@pahud
Copy link
Contributor

pahud commented Mar 22, 2023

Thank you for the report. I'll bring this up and discuss this with the core team.

@pahud pahud added p2 effort/small Small work item – less than a day of effort documentation This is a problem with documentation. and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Mar 22, 2023
@DFEvans
Copy link
Author

DFEvans commented Mar 23, 2023

After more chasing, I think whether this issue coming up depends on exactly how Python and tools around it are treating warnings - simply using -Werror doesn't trigger it, but with just the right setup, I can get it to trigger via Pytest.

I've created an example repo: https://github.com/DFEvans/cdk_invalid_char

It contains a single Github action, which fails with the above stack trace when trying to run a file which contains:

import aws_cdk as cdk

def test_thing():
    assert True

Apologies that there's still a bit of Poetry/Pytest stuff around the problem - I've not been able to pin down the specific set of warning commands that would get a plain python test.py to show the issue.

@ppena-LiveData
Copy link

ppena-LiveData commented May 31, 2023

NOTE: aws_cdk/aws_cloudwatch/__init__.py in aws-cdk-lib v2.81.0 has the same problem. Running python -Werror, I see this:

Python 3.11.3 (tags/v3.11.3:f3909b8, Apr  4 2023, 23:49:59) [MSC v.1934 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import aws_cdk
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\git\cdk-utils\.venv\Lib\site-packages\aws_cdk\__init__.py", line 33241, in <module>
    from . import aws_apigateway
  File "C:\git\cdk-utils\.venv\Lib\site-packages\aws_cdk\aws_apigateway\__init__.py", line 1742, in <module>
    from ..aws_certificatemanager import ICertificate as _ICertificate_c194c70b
  File "C:\git\cdk-utils\.venv\Lib\site-packages\aws_cdk\aws_certificatemanager\__init__.py", line 227, in <module>
    from ..aws_cloudwatch import (
  File "C:\git\cdk-utils\.venv\Lib\site-packages\aws_cdk\aws_cloudwatch\__init__.py", line 1
    '''
    ^^^
SyntaxError: invalid escape sequence '\|'

NOTE: make sure to delete a __pycache__ directory if it exists before attempting the above, otherwise Python will use the existing *.pyc file and won't try to re-load the file so you won't get the SyntaxError.

@ppena-LiveData
Copy link

This seems to be caused by the generated code that puts the documentation at the top of the module (so maybe this is really a jsii issue?). The fix seems to be as simple as putting an r at the top before the ''', i.e. make it a raw string, so that way Python doesn't try to interpret the backslashes as escape characters.

@jrobbins-LiveData
Copy link

@pahud, any word on this issue?

@toxygene
Copy link

I'm getting similar warnings with Python 3.12, aws-cdk 2.117.0.

[...]/aws_cdk/aws_cloudwatch/__init__.py:1: SyntaxWarning: invalid escape sequence '\\|'
[...]/aws_cdk/aws_secretsmanager/__init__.py:1: SyntaxWarning: invalid escape sequence '\`'

@bruceduhamel
Copy link

I've run into this issue on python 3.11, aws-cdk-lib 2.138.0.

Notably, if I install aws-cdk-lib with poetry, then run pytest , it generates warnings about invalid escape sequences.

If I then run pip uninstall -y aws-cdk-lib followed by pip install aws-cdk-lib, then pytest runs without warnings.

Is there any plan to fix this?

@pahud
Copy link
Contributor

pahud commented Jun 4, 2024

moving to jsii

@pahud pahud transferred this issue from aws/aws-cdk Jun 4, 2024
@kaizencc
Copy link
Contributor

kaizencc commented Jun 5, 2024

this is a tough one. it's not as simple as treating the entire string as a raw string in jsii (i.e. r''') because there are characters we'd like to escape, like \n. Simply changing the 2 instances where this is happening would be the path of least resistance but would not address the actual problem and open us to the same issue in the future. still looking into this one, but i don't have a magic solution yet.

@mergify mergify bot closed this as completed in #4538 Jun 10, 2024
mergify bot pushed a commit that referenced this issue Jun 10, 2024
Fixes #4532. Succinctly, the issue is that the README is copied over into the `__init__.py` file as a python comment. And then python warns that things like `\|` and `\'`, which while not often, do organically and correctly show up in markdown syntax, are invalid escapes. Some people who have set their python config to error on warnings end up erroring on this. The solution is to mark the README string as a raw string `r'''` so python does not try to register the escapes.

I'm not sure how to test this in code in this PR. I have done the following to make sure that this works:

I copied the repro repo from #4532 [here](https://github.com/kaizencc/cdk_invalid_char/blob/main) and got it to show the warning locally. then, I updated the actual file manually from `'''` to `r'''` and got `pytest -W error` to give me a thumbs up. So that shows that changing `'''` to `r'''` does not expect the escaped characters to be valid.

I tested my local jsii-pacmak by using it to package a module in `aws-cdk`, and unzipped the python package and confirmed that the `r'''` raw string indicator shows up for the README string in `__init__.py`, and nothing else.

These two combined confirms for me that this solution will work. Again I'm not sure of the best way to test that in jsii-pacmak.

---

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
Copy link
Contributor

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. documentation This is a problem with documentation. effort/small Small work item – less than a day of effort jsii p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants