Skip to content
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

schema: Introduce CoreModuleSchemaForConstraint #129

Merged
merged 5 commits into from
Aug 9, 2022

Conversation

radeksimko
Copy link
Member

@radeksimko radeksimko commented Aug 4, 2022

This is to enable hashicorp/terraform-ls#993


Implementation Notes

I chose to pull all known versions and generate a list of versions which we can check the constraints against. They are re-generatable at any time via go generate ./schema. This is more or less in-line with our longer term plans to have TF Core/CLI expose machine-readable schemas for core and publishing it somewhere for each version.

While working on this I also realised there's a few edge cases we didn't previously cover, which became more obvious now:

  • what happens if the user has too old version/constraint we don't have schema for? (<0.12)
  • what happens if the user has too new version/constraint we don't have schema for? (>= 1.3) - this is esp. important since we may not always have the list up-to-date and not all users may be on the latest version of LS while also consuming the latest version of Terraform

For both of these edge cases we return an error (which is what we did before), but given that we also now expose the lowest and highest (supported) version, we allow LS to pick either of those based on "proximity" to the version or constraint - e.g. if the user is running 0.5.0 we pick 0.12.0 as that's closest. If they run (hypothetical) 1.5.0, we pick 1.3.0.

@radeksimko radeksimko added the enhancement New feature or request label Aug 4, 2022
@radeksimko radeksimko force-pushed the f-core-schema-by-constraint branch from 885c1c3 to 20b8c89 Compare August 4, 2022 19:36
@radeksimko radeksimko self-assigned this Aug 4, 2022
@radeksimko radeksimko force-pushed the f-core-schema-by-constraint branch from 20b8c89 to e8c42f3 Compare August 5, 2022 10:45
@radeksimko radeksimko force-pushed the f-core-schema-by-constraint branch 2 times, most recently from e69a06e to 3381bbe Compare August 5, 2022 16:55
@radeksimko radeksimko force-pushed the f-core-schema-by-constraint branch from 3381bbe to 86bd319 Compare August 5, 2022 17:19
@radeksimko radeksimko marked this pull request as ready for review August 5, 2022 17:27
@radeksimko radeksimko requested a review from a team August 5, 2022 17:27
Copy link
Member

@dbanck dbanck left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

Only the gen_test.go was a bit confusing to me

@radeksimko radeksimko merged commit 573e228 into main Aug 9, 2022
@radeksimko radeksimko deleted the f-core-schema-by-constraint branch August 9, 2022 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants