Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

chore!: remove default org state#134

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

Open
aslilac wants to merge3 commits intomain
base:main
Choose a base branch
Loading
fromlilac/no-default-org-state

Conversation

aslilac
Copy link
Member

as an alternative to this, you should just do...

data"coderd_organization""default" {is_default=true}

...and refer tocoderd_organization.default.id when needed.

What we're currently doing is nottruly using the default organization, and hides details from the user that I think ultimately make the experience more dangerous and confusing.

Copy link
Member

@ethanndicksonethanndickson left a comment

Choose a reason for hiding this comment

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

This is a breaking change, and should only get added when we're ready for the 1.0 release.

Computed: true,
Optional: true,

Choose a reason for hiding this comment

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

This should beRequired now, and also on all the other resources.

@ethanndicksonethanndickson changed the titlechore: remove default org stateDNM: chore!: remove default org stateNov 12, 2024
@aslilac
Copy link
MemberAuthor

according to semver, every 0.0.x release is supposed to be considered "breaking".

we could even do 0.1.x, but I think this is something we should get feedback from long before 1.0, not afterwards.

@ethanndickson
Copy link
Member

ethanndickson commentedNov 13, 2024
edited
Loading

That's fair - I just don't want to get in the habit of regularly breaking people's Terraform configs. My other hesitance is that this becomes annoying for the majority of single organization deployments. (Maybe a multi-org use-case ends up having multiple Terraform configs too, it's not unreasonable for different template admins to handle their IaC independently? Can't say for sure)

I feel like we could make the experience 'safer' without sacrificing any of the convenience of the default. What would be your thoughts on keeping thedefault_organization_id attribute on the provider optional, but returning a custom error if both it, and the organization id on the resource, weren't supplied? That way there'd be no implicit org selection.

@aslilac
Copy link
MemberAuthor

  • Allowing them to set a global default adds a lot of complexity to the provider
  • It reduces our ability to validate the config during a plan (becauseorganization_id would have to be optional on every resource that takes one)
  • It's really not that hard for them to just add...
data"coderd_organization""default" {is_default=true}resource"...""..." {organization_id=coderd_organization.default}

It's a tiny bit of extra code for them, but avoids a lot of complexity, confusion, and magic for everyone.

@ethanndickson
Copy link
Member

I don't really agree that it adds a huge amount of complexity to the provider, user-facing or implementation wise, but I agree, it would be incredibly annoying forterraform plan to not pick up that you haven't set an org ID in either place. I've decided the implicit org selection (the first the user is a member of) is pretty bad, so yeah, let's just get rid of it for now, and then we can include it in the next release, where we're already bumping the minor version.

aslilac reacted with thumbs up emoji

@ethanndicksonethanndickson changed the titleDNM: chore!: remove default org statechore!: remove default org stateDec 19, 2024
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ethanndicksonethanndicksonethanndickson left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@aslilac@ethanndickson

[8]ページ先頭

©2009-2025 Movatter.jp