Before I go too far into making tests work and cleaning up the PR, I'd like@sreya to take a look and make sure this approach is sound. Thanks

@f0ssel, this is great work. 👍🏼

This PR is a bit out of scope, but do you think we can also pull all template values, including metadata, usingcoder template pull or probably a new commandcoder template info? This can be a separate PR, but it would improve the UX to get the current values using the CLI before pushing an update. It would be even better to have a--output json output for this.

@matifali That all seems possible in a separate issue from my understanding but is out of scope for this PR. Can look into it next though.

@f0ssel sounds fine.

@ammario Thanks for the review. There will be some code reduction oncecoder template create is removed, in the mean time it simplifies the create command at the cost of making the push command more complicated. If we feel this isn't a good solution I'm more than OK with scrapping this and going in a different direction, because I too am not super pleased with the lack of code reduction in this PR.

If API and code clarity are what we are really valuing here, I would almost suggest the opposite of this approach and make
"dumber" distincttemplate create|edit-metadata|update commands that don't do so much, have less overlapping flags, and requires the user to use each for the use case instead of a mega command like push that does everything. My understanding of the goal was really to deprecate create so we have one less command to maintain while giving users the simplicity of one command for any and all template updates. Could you give me some insight into which direction you would prefer here?

In addition to other comments I'll address, I can look into the entitlement stuff getting checked on the server, I also felt the issue with separation of concerns here.

A major perceived benefit of combining the commands was idempotency. For example, in CI, you could write a script that iterates all folders and runscoder templates push in them without consideration as to whether a template already exists.

I like to think the best solution for the user and the codebase is the same thing. As an aside, we mistakenly call a lot of these fields "metadata" when in fact there's nothing "meta" about it. We should instead call it the template configuration.metadata is typically read-write-able labels meant to be consumed by third party software.

What do think about combiningtemplate create intotemplate push but then keeping a separate command for configuration. That configuration command could betemplate config, which could also print the configuration to stdout as well as enable changing it.

We call "config","metadata",whatever "settings" in the UI. So we should probably either follow that or change it everywhere.

A major perceived benefit of combining the commands was idempotency. For example, in CI, you could write a script that iterates all folders and runs coder templates push in them without consideration as to whether a template already exists.

@ammario So there's where the difficulty is in these changes lie - we want to support flags that the create API accepts, but we don't always use the template create API because it could just be a version update. In the case where we update, we need to then use the template edit API to apply those flags as a supplement. This is where the template edit code comes into thepush command, even if we keep it as a separate command itself. So forpush we are kind of left with these options:

1. Makepush andcreate command "dumber" - Drop the flags that rely on the create / edit API forpush altogether, only push new template versions. This is the existing behavior in main. We would then drop those settings flags oncreate and make it "dumber" but now a true subset ofpush.
2. Makepush command "smarter" - Move all thecreate command flags intopush command, use the edit API to apply those flags when not creating and only updating. This is how this current PR works. This makespush a superset of the existingcreate with all it's current "settings" flags. I've also added in the edit flags because why not - we are already calling the edit API. But these could be removed from this PR to make it slightly cleaner code.
3. Makepush only apply "settings" flags on create - This is tricky behavior for the end user, because you would need to silently not apply those flag values to ensure idempotency because errors when supplied "settings" flags on non-create executions would mean you couldn't run the same script twice. This would be the "cleanest" code but not a very good UX imo.

All 3 of these template commands do pretty different things and rely on different APIs, so we just need to figure out where to unify them. My favorite is#1, which comes at the cost of a less usefulcreate command but moves those concerns into theedit command and makes the cleanest API and code IMO. Would like to know your thoughts here as well.

Secondly, I looked and after removing thecreate command sometime in the future it makes the code diff lose about 800 loc, which then balances out this new additional code. Idk how soon of a timeline we remove it though without hurting existing customer usability.

I've mocked up a PR of what option 1 would look like here -#11390 basically we strip the settings flags from create and combine as much code as possible between the two commands for now. This would allow us to lead users to only using thepush andedit|settings commands for all of their needs as well as removes the entitlements code required increate.

OK thanks for the explanation. I'll leave a comment within#11390.

Closing in favor of#11390