Skip to content

Comments

Fix cloudfront usage#467

Merged
Szpadel merged 1 commit intorocky-developfrom
dev-fixcloudfront
Apr 15, 2025
Merged

Fix cloudfront usage#467
Szpadel merged 1 commit intorocky-developfrom
dev-fixcloudfront

Conversation

@MariuszJozwiak
Copy link
Contributor

There was missing rule that execute cloudfront every time even when it should be skipped.

@Szpadel
Copy link
Member

Szpadel commented Mar 18, 2025

Why disable cloudfront discovery?
This does not look like good approach to this issue, why not just disable setting up cloudfront to magento?
Like here: https://github.com/mageops/ansible-infrastructure/blob/rocky-master/group_vars/all.yml#L1153-L1175

@MariuszJozwiak MariuszJozwiak force-pushed the dev-fixcloudfront branch 2 times, most recently from 0aa3463 to 4d7b783 Compare March 20, 2025 07:27
@MariuszJozwiak MariuszJozwiak marked this pull request as draft March 20, 2025 08:03
@mageops mageops deleted a comment from Szpadel Mar 20, 2025
@MariuszJozwiak MariuszJozwiak force-pushed the dev-fixcloudfront branch 2 times, most recently from 9a5e82a to c221257 Compare March 20, 2025 09:14
@MariuszJozwiak MariuszJozwiak marked this pull request as ready for review March 20, 2025 11:18
Copy link
Member

@Szpadel Szpadel left a comment

Choose a reason for hiding this comment

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

Are you sure this works as intended?

value: "https://{{ aws_cloudfront_distribution_domain | default('') }}/"
default: "{{ aws_cloudfront_domain_aliases | default([]) | length > 0 }}"
enabled: "{{ aws_cloudfront_distribution_domain | default(false) is string }}"
enabled: "{{ aws_cloudfront_distribution_domain is string and aws_cloudfront_distribution_create | default(false) | bool }}"
Copy link
Member

Choose a reason for hiding this comment

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

why do we need here this | default(false) | bool?

what case is not handled with? aws_cloudfront_distribution_domain is string and aws_cloudfront_distribution_create

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed and tested

@Szpadel Szpadel merged commit 94d1ab7 into rocky-develop Apr 15, 2025
MariuszJozwiak pushed a commit that referenced this pull request May 13, 2025
MariuszJozwiak pushed a commit that referenced this pull request May 13, 2025
MariuszJozwiak pushed a commit that referenced this pull request May 13, 2025
MariuszJozwiak pushed a commit that referenced this pull request May 16, 2025
MariuszJozwiak pushed a commit that referenced this pull request May 19, 2025
MariuszJozwiak pushed a commit that referenced this pull request May 19, 2025
MariuszJozwiak pushed a commit that referenced this pull request May 19, 2025
MariuszJozwiak pushed a commit that referenced this pull request May 19, 2025
MariuszJozwiak pushed a commit that referenced this pull request May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants