Skip to content
This repository was archived by the owner on May 15, 2025. It is now read-only.

fix(filebrowser): only require agent_name when not on subdomain #299

Merged
merged 4 commits into from
Sep 27, 2024

Conversation

matifali
Copy link
Member

@matifali matifali commented Sep 24, 2024

Ensure that agent_name defaults to an empty string, and introduce
validation to require it when subdomain is true. This aligns with
expected behavior and prevents misconfiguration.

generated by aicommit

Ensure that agent_name defaults to an empty string, and introduce
validation to require it when subdomain is true. This aligns with
expected behavior and prevents misconfiguration.
@matifali matifali self-assigned this Sep 24, 2024
@matifali matifali requested a review from phorcys420 September 24, 2024 06:05
Simplify test cases by removing unnecessary `agent_name`
variable entries. Adjust validation condition for clarity.
@phorcys420
Copy link
Member

phorcys420 commented Sep 24, 2024

hey @matifali, couldn't we just grab it somehow from the agent id, or pass the full agent object instead of the id?
i'll still review l8r though ;-)

@matifali matifali requested review from code-asher, kylecarbs, Parkreiner and bcpeinhardt and removed request for kylecarbs and code-asher September 24, 2024 17:25
@matifali
Copy link
Member Author

@phorcys420 We do not have any way to get the agent name from the id within Terraform yet. We are using the same approach in https://registry.coder.com/modules/jetbrains-gateway module

@matifali matifali enabled auto-merge (squash) September 27, 2024 09:34
Copy link
Member

@Parkreiner Parkreiner left a comment

Choose a reason for hiding this comment

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

These look like really straightforward changes. Approving

@matifali matifali merged commit 1628087 into main Sep 27, 2024
3 checks passed
@matifali matifali deleted the maa/dotfiles branch September 27, 2024 15:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants