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

feat: Configure target ECS cluster ARN #15

Merged
merged 2 commits into from
May 17, 2021
Merged

feat: Configure target ECS cluster ARN #15

merged 2 commits into from
May 17, 2021

Conversation

oliversalzburg
Copy link
Contributor

Allows configuring a specific ECS cluster to monitor, using the --cluster=ARN CLI argument.

Fixes #13

If someone could point out how to adjust the log line during startup to include this setting, I'd be happy to include it.

Copy link
Member

@Limess Limess left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

We don't spend a lot of time maintaining this or checking for notifications, but this looks like a sensible improvement so I'll try to help you get it in.

It'd be useful if you were to also update the README with the new option. I'm aware that's a little bit messy right now so don't worry about placement too much.

discoverecs.py Outdated Show resolved Hide resolved
discoverecs.py Outdated Show resolved Hide resolved
discoverecs.py Outdated Show resolved Hide resolved
@oliversalzburg
Copy link
Contributor Author

I've made the changes to the best of my knowledge. I'll test these in my own setup and fix any issues that come up. Otherwise, I'd also appreciate a second review anytime :)

Copy link
Member

@Limess Limess left a comment

Choose a reason for hiding this comment

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

Another small change to ensure the current behaviour is the same but looks good

discoverecs.py Outdated Show resolved Hide resolved
discoverecs.py Outdated Show resolved Hide resolved
@Limess Limess merged commit 9721450 into signal-ai:master May 17, 2021
@Limess
Copy link
Member

Limess commented May 17, 2021

Merged, thanks again!

@oliversalzburg oliversalzburg deleted the feat/cluster-selection branch May 17, 2021 09:06
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.

Support cluster selection
2 participants