- Notifications
You must be signed in to change notification settings - Fork22
feat: add resources_monitoring field to agent#331
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Blocking this from accidental merge since we already have one approval, but some work is still required for validation.
@mtojek@dannykopping - I improved the validation and added test cases for all the main cases, should be way better now. ✅ |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
I added some more cases to ensure we test all new fields and options. Just a point remaining to discuss based on what we want to do around the duplicates validation. More details on this comment but globally I copy pasted metadata - still if there's any better place I am open to change it. Otherwise I tested the tf-provider with coder/coder and it works exactly as we want - so I'm happy about the current behavior. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
matifali commentedFeb 7, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I have two suggestions.
WDYT@johnstcn as we recently spoke about versions for our provider? |
I'll indeed defer answers to your questions to@johnstcn - not sure if this is doable and how , but happy to implement the check if we want to. About the versioning, both are fine to me. I already tested it a lot with the Coder code we will deploy but a rc can be good too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nice!
It should be possible to hit the
I'm fine with RC releases. It's a little annoying to have to cherry-pick each time, but it's not the end of the world. |
1f95807
intomainUh oh!
There was an error while loading.Please reload this page.
As required perthis issue - we need to add a new field in terraform-provider-coder , more specifically in the agent.