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

cyclic import on ServerlessAppPlugin with 1.60.0 #2975

Closed
alexrashed opened this issue Feb 28, 2023 · 3 comments
Closed

cyclic import on ServerlessAppPlugin with 1.60.0 #2975

alexrashed opened this issue Feb 28, 2023 · 3 comments
Labels

Comments

@alexrashed
Copy link

Description

With the release of 1.60.0, it is not possible anymore to import from samtranslator.plugins.application.serverless_app_plugin due to a cyclic import introduced with be58556.

Steps to reproduce

I used the following script to verify my tests (and to identify the commit which introduced the issue using it as a run script for git bisect):

#!/bin/bash
set -eo pipefail
rm -rf .venv
python -m venv .venv
source .venv/bin/activate
pip install -e .
python -c "from samtranslator.plugins.application.serverless_app_plugin import ServerlessAppPlugin"

Observed result

This script mentioned above fails on the v1.60.0 tag, as well as on the latest commit on develop, with the following error:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/localstack/Repos/serverless-application-model/samtranslator/plugins/application/serverless_app_plugin.py", line 11, in <module>
    from samtranslator.intrinsics.actions import FindInMapAction
  File "/home/localstack/Repos/serverless-application-model/samtranslator/intrinsics/actions.py", line 5, in <module>
    from samtranslator.model.exceptions import InvalidDocumentException, InvalidTemplateException
  File "/home/localstack/Repos/serverless-application-model/samtranslator/model/__init__.py", line 7, in <module>
    from samtranslator.intrinsics.resolver import IntrinsicsResolver
  File "/home/localstack/Repos/serverless-application-model/samtranslator/intrinsics/resolver.py", line 4, in <module>
    from samtranslator.intrinsics.actions import Action, GetAttAction, RefAction, SubAction
ImportError: cannot import name 'Action' from partially initialized module 'samtranslator.intrinsics.actions' (most likely due to a circular import) (/home/localstack/Repos/serverless-application-model/samtranslator/intrinsics/actions.py)

Expected result

With b3b6c79, the import does not fail (which I would expect).

Additional environment details

  1. OS: Ubuntu 22.04, Python 3.10
  2. If using the SAM CLI, sam --version: -
  3. AWS region: -
@alexrashed alexrashed added stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. type/bug labels Feb 28, 2023
@aahung
Copy link
Contributor

aahung commented Feb 28, 2023

Thanks for reporting. Looks like when we added type hints, it reveals that .models and .intrinsics are importing from each other. This should be avoided as .models.__init__ needs to focus on models not other behavior.

It isn't an issue if samtranslator.translator is directly imported. (as long as samtranslator.intrinsics.resolver is imported first it is not an issue).

@aahung aahung removed the stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. label Feb 28, 2023
@aahung
Copy link
Contributor

aahung commented Feb 28, 2023

List of modules that cannot be imported alone:
image

@aahung
Copy link
Contributor

aahung commented Feb 28, 2023

Fixed in https://github.com/aws/serverless-application-model/releases/tag/v1.60.1

@aahung aahung closed this as completed Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants