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

Add .env for specifying aws logging specifics #164

Merged
merged 2 commits into from
Dec 13, 2023
Merged

Conversation

jayjb
Copy link
Contributor

@jayjb jayjb commented Dec 12, 2023

Proposed changes

This commit allows a .env file that is used to define some of the AWS logging specifics. We do have defaults so it is not required.

Types of changes

What types of changes does your code introduce to this repository?
Put an x in the boxes that apply

  • New feature (non-breaking change which adds functionality)
  • Documentation Update (here)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • Lint and unit tests pass locally with my changes (if applicable)
  • I have run pre-commit (pre-commit in the repo)

Copy link
Contributor

@wleightond wleightond left a comment

Choose a reason for hiding this comment

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

I've suggested one small README.md fix

Approving to unblock you

README.md Outdated Show resolved Hide resolved
@@ -7,8 +7,8 @@ services:
logging:
driver: awslogs
options:
awslogs-region: eu-west-1
awslogs-group: canarytokens-dev
awslogs-region: ${CANARYTOKENS_LOGGING_REGION:-eu-west-1}
Copy link
Contributor

Choose a reason for hiding this comment

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

My first thought was that we may want to use the standard CANARY_ prefix, but as this is not being included in the settings objects I suspect it's better as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmmm its a good thought to raise. I hadn't considered it. I'll leave for now.

Co-authored-by: W. Leighton Dawson <[email protected]>
@jayjb jayjb merged commit c3e6f0b into master Dec 13, 2023
1 check passed
@jayjb jayjb deleted the add_env_for_logging branch December 13, 2023 05:42
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