Skip to content

Conversation

@k-a-il
Copy link
Contributor

@k-a-il k-a-il commented Jan 14, 2026

Motivation

The PR #13597 introduced changes which caused side-effects in another repository. This PR fixes problematic import by moving it to function level.

Changes

  • Move import of CFN plugin manager to function level

Relates to FLC-80

@k-a-il k-a-il requested review from bentsku and dfangl January 14, 2026 18:28
@k-a-il k-a-il self-assigned this Jan 14, 2026
@k-a-il k-a-il added semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes labels Jan 14, 2026
@k-a-il k-a-il changed the title IaC: move CFN import in catalog to function level IaC: move CFN plugin manager to function level Jan 14, 2026
@k-a-il k-a-il changed the title IaC: move CFN plugin manager to function level IaC: fix problematic CFN plugin manager import Jan 14, 2026
Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for jumping on this!

We'll need to think more thoroughly about the upstream resources as well with this, and maybe deferring this import even further by making this a cached property instead of a static method we call once. But this can be a follow-up, let's unblock ourselves for now!

edit: there's also the issue of importing service code outside of the service itself, but we can think about that later. If we only have CloudFormation calling this code, then it would be fine. More argument towards deferring the import until CloudFormation itself calls this code/makes use of it 👍

@github-actions
Copy link

Test Results - Preflight, Unit

23 069 tests  ±0   21 224 ✅ ±0   6m 55s ⏱️ -38s
     1 suites ±0    1 845 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 10695c5. ± Comparison against base commit 7517af4.

@k-a-il
Copy link
Contributor Author

k-a-il commented Jan 14, 2026

LGTM! Thanks for jumping on this!

Thank you and @dfangl for identifying the issue!

We'll need to think more thoroughly about the upstream resources as well with this, and maybe deferring this import even further by making this a cached property instead of a static method we call once. But this can be a follow-up, let's unblock ourselves for now!

Good point, will follow-up on this 👍

@github-actions
Copy link

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   2m 52s ⏱️ -4s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 10695c5. ± Comparison against base commit 7517af4.

@github-actions
Copy link

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 35m 36s ⏱️
5 590 tests 5 022 ✅ 568 💤 0 ❌
5 596 runs  5 022 ✅ 574 💤 0 ❌

Results for commit 10695c5.

@k-a-il k-a-il merged commit 2c809d8 into main Jan 14, 2026
54 checks passed
@k-a-il k-a-il deleted the fcl-80-fix-import-issue-catalog branch January 14, 2026 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes semver: minor Non-breaking changes which can be included in minor releases, but not in patch releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants