-
Notifications
You must be signed in to change notification settings - Fork 48
AAP-46677-updated-terraform.py #194
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
base: main
Are you sure you want to change the base?
AAP-46677-updated-terraform.py #194
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #194 +/- ##
==========================================
+ Coverage 73.00% 80.77% +7.76%
==========================================
Files 16 23 +7
Lines 1015 1737 +722
Branches 182 343 +161
==========================================
+ Hits 741 1403 +662
- Misses 243 289 +46
- Partials 31 45 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 21s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 16s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 12s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 14s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 27s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 20s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 14s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 32s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 08s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 05s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 2m 50s |
changelogs/fragments/20250620-modules-terraform-update-workspace-logic.yml
Outdated
Show resolved
Hide resolved
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 41s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 17s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 15s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 42s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 56s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 40s |
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 17s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a lot of concerns about this PR. The parsing logic currently does not account for:
- nested blocks
- jsonencode
- heredoc syntax
- workspace tags
- terraform config written in json
In addition, we're opening and reading every file in the directory and then looping over every single character in every one of those files, and that's being done every time this module is used.
Can we not just use workspace list
to figure out what workspace terraform thinks is configured? I'm not clear on what problem this PR is trying to solve.
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 22s |
SUMMARY
Please refer to https://issues.redhat.com/browse/AAP-46677.
ISSUE TYPE
COMPONENT NAME
terraform.py
ADDITIONAL INFORMATION
f the workspace is given in the playbook - Then use that version
if there is a mismatch between the playbook and the one in the cloud config then an error would likely occur
If the workspace is not given and cloud block is present - Then use workspace from cloud block version
If the workspace is not given and cloud block is not used - Then use CLI version (existing logic).
Also I have added a TF file parser which removes comments from the TF file.