chore: warn when coder_agent dir breaks Desktop file sync#507
Conversation
johnstcn
left a comment
There was a problem hiding this comment.
Suggestion: you could also add a diag.Warn in agentResource() so that this will show up in the Terraform log output. May be too noisy though.
Ooo that sounds good, and I think that would obviate the need for coder/coder#24951? Feels to me like having the warning in the provider makes more sense. |
ddac3ee to
8c8b970
Compare
|
Should we also mark this as a deprecated attribute? |
|
I wasn't sure if we wanna fully deprecate the attribute as part of this ticket, was expecting we would have a separate push to properly deprecate and clean up which will take a couple of releases. @matifali I can adjust the warning to say it's deprecated as well if you want |
|
Deprecate means we start emitting a warning in terraform build logs, but it will keep working until next major release when we remove it. |
|
Okay, will update to add a deprecation warning too |
0464866 to
1d171c6
Compare
Add a ValidateDiagFunc on the dir schema attribute that emits a diag.Warning when dir is set to a value other than $HOME. Also add a doc callout in the attribute description for the Terraform Registry.
1d171c6 to
ec5ff5c
Compare
|
/coder-agents-review |
|
I'm happy about the deprecation of this attribute, but it's potentially a pretty big breaking change for DX. Currently we use Re: breakage, for instance in Coder Agents |
|
This is the first phase where we started emitting warnings to discourage it's use. It should still work the same as before. It's a good chance to talk about possible alternatives here.
|
|
Also this repo need more verbose agent instructions to check any use of attributes in the coder/coder repo too so LLMs can have more context to suggest better completions. |
|
@matifali I think it isn't a great user/customer experience for us to deprecate it without making it clear what our future plans are. Will we support the use-case(s) through some other name? Will we support part of the use-cases, or none at all? Will we split it out into multiple parameters? Customers updating the provider will wonder what they should do now that it's deprecated, and they can't find any answers. To make matters worse the reasoning can't even be discovered in the changelog as this is riding behind the "desktop file sync warning". |
|
These are all great points.
The recommendation for 2, is to use our modules from registry.coder.com which allows explicitly setting |
Adds a warning to the
dirattribute on thecoder_agentresource for Coder Desktop file sync compatibility.Two mechanisms:
~> Warningblock in thedirschema description, rendered on the Terraform Registry.diag.WarninCreateContextandReadWithoutTimeoutthat fires duringterraform plan/applywhendiris set to a non-$HOMEvalue. This warning flows through Terraform output into Coder build logs.Refs DEVEX-227