Skip to content

Conversation

@ahummel25
Copy link
Contributor

Description

Adding logging_configuration property to enable Cloudwatch logging for step functions

Motivation and Context

Need ability to utilize Cloudwatch logging for step functions

Breaking Changes

How Has This Been Tested?

  • I have tested and validated these changes using one or more of the provided examples/* projects

@ahummel25
Copy link
Contributor Author

Looks like my IDE took some liberties reformatting the README. Let me try fixing that.

@ahummel25
Copy link
Contributor Author

Actually the README looks completely fine in preview mode

Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

Please update the code a bit.

@ahummel25
Copy link
Contributor Author

@antonbabenko Addressed PR comments. Let me know if I've missed the mark on anything.

@ahummel25 ahummel25 changed the title Feature/configure cloudwatch logging Configure cloudwatch logging for step functions module Mar 18, 2021
@ahummel25 ahummel25 changed the title Configure cloudwatch logging for step functions module feat: Configure cloudwatch logging for step functions module Mar 18, 2021
@atrakic
Copy link

atrakic commented Mar 25, 2021

Looking fwd to see this PR merged.

@slewis-bd
Copy link

Any chance this PR can be reviewed @antonbabenko? Looking forward to this being merged.

@svenlito
Copy link

svenlito commented Apr 2, 2021

@ahummel25 Could resolve the conflict, please?

@ahummel25
Copy link
Contributor Author

ahummel25 commented Apr 2, 2021

@svenlito Done.

@antonbabenko I've resolved the merge conflicts, but not sure why the Pre-Commit hook is failing. Can you please advise?

Passes all pre-commit checks locally.

pre-commit run --color=always --show-diff-on-failure --all-files
Terraform fmt............................................................Passed
Terraform validate.......................................................Passed
Terraform docs...........................................................Passed
Terraform validate with tflint...........................................Passed
Check for merge conflicts................................................Passed

@bryantbiggs
Copy link
Member

@ahummel25 if you update the .github/workflows/pre-commit.yml as shown here, your checks should pass - https://github.com/terraform-aws-modules/terraform-aws-autoscaling/pull/139/files#diff-ac5eead3f3ce4863c524fff031a87b7aecccb4a0493df087a4e1c704a1505036R97

its the terraform docs that we are still unfortunately fighting with at the moment

@ahummel25
Copy link
Contributor Author

@bryantbiggs Just did so. Looks like it's still failing. Any ideas?

@bryantbiggs
Copy link
Member

so that fixed the pipeline but it looks like you haven't run the pre-commit hook on the codebase so you have a diff
image

@ahummel25
Copy link
Contributor Author

I just ran pre-commit run -a locally and nothing got updated. Is that the right command to run?

@bryantbiggs
Copy link
Member

thats correct, but I suspect that your local version of terraform-docs is out of date

$ terraform-docs --version
> terraform-docs version v0.12.1 darwin/amd64 BuildDate: 2021-03-29T18:15:47+0100

@ahummel25
Copy link
Contributor Author

Thanks @bryantbiggs. That fixed it.

@antonbabenko antonbabenko force-pushed the feature/configure-cloudwatch-logging branch from 6d5cfc2 to ceea40d Compare April 7, 2021 10:42
@antonbabenko antonbabenko merged commit 0fe6b2e into terraform-aws-modules:master Apr 7, 2021
@antonbabenko
Copy link
Member

Thanks @ahummel25 !

v1.3.0 has been just released.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants