Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add login_type to coder_workspace_owner data source #235 #287
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 login_type to coder_workspace_owner data source #235 #287
Changes from 1 commit
ab53274
bf8e5d0
a32460b
6f96982
fb69196
1e4b8fb
210847d
2f3f734
63607a3
3eb23d2
241f90a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@mtojek should we instead set
login_type
to an empty string if Coder does not set the required env?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.
Perhaps at the very least, drop a
diag.Warn
?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.
This seems reasonable and consistent with other ENV vars. I would start with empty string, we can always revisit it in the future and replace it with
none
.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.
Great, what about the warn?
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.
Adding a
diag.Warn
if theCODER_WORKSPACE_OWNER_LOGIN_TYPE
environment variable is not present or empty would be a good indication to the user that they have a potential mismatch of the Coder version and provider version.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.
Sorry, but taking a look to the
diag
package there is notWarn
method.https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/diag
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.
Sorry! I should have been more specific -- you can append a
diag.Diagnostic
with severitydiag.Warning
to the second return argument of typediag.Diagnostics
.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.
Done, this is the first time I have to deal with the
diags
, so please let me know if this solution is correct 🙏🏼 .