chore: Improve imports handling in generators #4043

sfc-gh-asawicki · 2025-09-19T11:59:13Z

Test import processing behavior
Introduce HasPreambleModel to constrain generator models
Extract the common preamble model and template
Use the common preamble in all generators set up with the common constructor
Add generator name and version to the common preamble
Remove old preamble models and templates
Remove duplicated package logic
Remove the SomeFunc
Add build exclude directive to all generators set up with the common constructor
Generate missing grant-related resource assertions
Regenerate all generators based on the commons generator
Generate missing account resource parameters assertions
Generate missing schema resource parameters assertions
Generate missing resource show output assertions for functions and procedures
Add missing "adjusted manually" comments in various generated files
Add missing objects to models lists
Remove the _ = sdk.Object{} unnecessary with the improved imports handling
Remove all additional imports handling logic

Next PRs:

Extract common setup and helpers for the imports expeiment tests
Use commons generator in other existing generators
Update generators documentation
Discuss and document version bumping

github-actions · 2025-09-19T12:55:57Z

Account-level tests failure for cbd72efacf887a927e468f8ff5b15e549e0dade6

github-actions · 2025-09-19T14:45:43Z

Integration tests success for cbd72efacf887a927e468f8ff5b15e549e0dade6

pkg/internal/genhelpers/imports_test.go

sfc-gh-jcieslak · 2025-09-22T07:06:09Z

pkg/internal/genhelpers/imports_test.go

+
+		out, err := genhelpers.AddImports("", src)
+
+		require.NoError(t, err)


Is there any case where we should expect errors being returned?

imports.Process can return error, so yes. I will add test for error being returned (in the follow-up PR).

sfc-gh-jcieslak · 2025-09-22T07:10:08Z

pkg/internal/genhelpers/templates/preamble.tmpl

@@ -0,0 +1,11 @@
+{{- /*gotype: github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/genhelpers.PreambleModel*/ -}}
+
+// Code generated by {{ .GeneratorName }} generator (v{{ .GeneratorVersion }}); DO NOT EDIT.


Question: From now on, every modification to any generator should bump its version, right?

I wanted to discuss this and document what we agree. I will add it to our tomorrow agenda.

sfc-gh-jmichalak · 2025-09-22T11:56:25Z

pkg/acceptance/bettertestspoc/assert/resourceparametersassert/schema_resource_parameters_gen.go

+// Parameter value checks //
+////////////////////////////
+
+func (s *SchemaResourceParametersAssert) HasDefaultDdlCollation(expected string) *SchemaResourceParametersAssert {


Why is it only 1 param?

As it was defined like this in objects_parameters_def but wasn't generated. It's not the full definition and it should be extended, I added it to SNOW-1501905 and I will leave a TODO later

Done in #4048

sfc-gh-jmichalak · 2025-09-22T12:00:43Z

pkg/internal/genhelpers/imports_test.go

+
+// TODO [next PR - SNOW-2324252]: extract common setup, template for easier test writing
+func Test_ExperimentWithImportsProcess(t *testing.T) {
+	t.Run("add standard imports", func(t *testing.T) {


Nit: use table tests.

yes, part of Extract common setup and helpers for the imports expeiment tests listed in the PR description

sfc-gh-jmichalak · 2025-09-22T12:12:21Z

pkg/acceptance/bettertestspoc/config/model/gen/model.go


-func ModelFromResourceSchemaDetails(resourceSchemaDetails genhelpers.ResourceSchemaDetails) ResourceConfigBuilderModel {
-	additionalImports := make([]string, 0)
+func ModelFromResourceSchemaDetails(resourceSchemaDetails genhelpers.ResourceSchemaDetails, preamble *genhelpers.PreambleModel) ResourceConfigBuilderModel {


Q: Why not just pass only the modelProvider to NewGenerator with preamble model embeeded before? I believe in NewGenerator we only call the modelProvider with the passed preamble, and the preamble itself is not used elsewhere.

But it is the modelProvider which constructs the model for each object, the preamble is not embedded earlier, that's why it's passed from the generator definition to the providerModel func (which does the embedding)

Continuation for: #4043 Use the common generator set up for the SDK generator: - Introduce the new logic for "generation parts" (all previous generators output a single file; SDK outputs several) - Part of the SDK is generated through a secondary generator (dto builders) - it is addressed in the subsequent PR (#4049) - Add filtering by generation part using env (similar to the existing object type env filtering example) - extracting it as a command line usable filter is addressed in the subsequent PR (#4050) - Move both filters to the generator commons - Change the test package to `genhelpers_test` to avoid the circular dependency - Use common preamble - Introduce `preprocessDefinition`, replacing the logic contained previously in the template executors - Extract temporary new fields to move the old template executors' logic to templates: - `PathToDtoBuilderGen` - pointing to the DTO builder gen - `StructsToGenerate` - containing all structs to be generated (unique logic was handled differently before; it was the easiest way) - `DtosToGenerate` - as above - `ObjectIdMethod` is a model to generate the ID() method for an SDK object; replaces the old logic - `ObjectIdMethod` is a model to generate the ObjectType() method for an SDK object; replaces the old logic - Revert whitespace handling changed in the recent semantic views change - Describe the proposed rework in a dedicated README - `sdk_definitions` file is needed for the compilation of the newly generated files Next PRs: - Rework dto builders generator - Add common filters to the main generator logic - TODO left in code, already addressed in the subsequent PR (#4050) - Update generators documentation - Extract a vararg constructor and use it in all generators (if this one is approved) - TODO left in code - Fix generation logic for the implementation mapping (check TODO in `streamlits_impl_gen.go`) - Use the new generator on all SDK-generated files (after DTO builder generator rework and filtering) - Remove `PathToDtoBuilderGen` - TODO left in code - Remove SDK template executors (after full regeneration) - multiple TODOs left in code - Move the current rework to the `example` directory - Decide if the object type is needed in the object assert definition (it looks unused) - TODO left in code - Update schema parameters definition and use in tests (result of #4043 (comment)) - TODO left in code - The templates may not have been updated with `conversionErrorWrapped` during the `convert` method signature change - TODO left in code - In SDK generator logic, split the definition from model (all other generators have this distinction, but in the SDK, the same object is used) - TODO left in code

@sfc-gh-mkowalski

🤖 I have created a release *beep* *boop* --- ## [2.8.0](v2.7.0...v2.8.0) (2025-10-06) ### 🎉 **What's new:** * add integration tests for semantic views ([#3978](#3978)) ([79482ea](79482ea)) * add semantic view resource - 1 ([#4029](#4029)) ([f0df2ab](f0df2ab)) * Add use_private_link_endpoint option to storage integrations ([#3913](#3913)) ([8853d9a](8853d9a)) ### 🔧 **Misc** * Add [@sfc-gh-mkowalski](https://github.com/sfc-gh-mkowalski) to the protected users list ([#4057](#4057)) ([d316ea2](d316ea2)) * Add common filtering flags to each generator ([#4050](#4050)) ([84568f0](84568f0)) * Adjust integration tests ([#4052](#4052)) ([d6e9e2c](d6e9e2c)) * Adjust tables documentation ([#4037](#4037)) ([f8416ee](f8416ee)) * Adjust warehouses after the changes in Snowflake ([#4038](#4038)) ([89d9da1](89d9da1)) * Bump driver to v1.17.0 ([#4053](#4053)) ([14f723f](14f723f)) * Generate DTO builders in SDK generator ([#4049](#4049)) ([1f2d9d8](1f2d9d8)) * Handle BCR 2025-06 ([#4036](#4036)) ([a009b16](a009b16)) * Improve bcr handling in tests ([#4042](#4042)) ([d21c1b3](d21c1b3)) * Improve imports handling in generators ([#4043](#4043)) ([cf7dcd3](cf7dcd3)) * Set dependabot target branch to dev ([#4056](#4056)) ([4b83f08](4b83f08)) * Stabilize acceptance tests ([#4044](#4044)) ([d15cbab](d15cbab)) * Use common generator setup in SDK generator ([#4048](#4048)) ([e0bc273](e0bc273)) ### 🐛 **Bug fixes:** * Bug fixes and documentation improvements ([#4045](#4045)) ([3c3819b](3c3819b)) * Handle REPLICABLE_WITH_FAILOVER_GROUPS in snowflake_object_parameter ([#4062](#4062)) ([9b28486](9b28486)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: terraform-provider-release[bot] <205196624+terraform-provider-release[bot]@users.noreply.github.com>

sfc-gh-asawicki added 30 commits September 19, 2025 13:56

Experiment with imports processing

2754b73

Add addimports helper

ae402cf

Use addimports (WIP)

ec1e1d4

Test addimports

326e3ef

Add common preamble template (without helpers and without usage)

9057a9a

Add preamble model constructor

8c3e941

Implement SomeFunc inside the PreambleModel

cbb0baa

Test unused explicit imports behavior

0f681ec

Replace copy-pasted preamble with the common one (WIP)

1ce61be

Remove all copy-pasted preamble templates

95bc336

Add named import tests

c635f53

Use the real preamble

9303abc

Fix interface

7a9fb7d

Hard code import for test

55ac2bc

Handle named imports too

6e0b1c7

Handle different receiver types

953e882

Use preamble in the generator constructor

5cdb864

Regenerate resource model builders

eb959ec

Regenerate data source model builders

9d698f8

Regenerate provider model builders

8850dde

Regenerate object assertions

8c7c8e2

Regenerate object parameter assertions

bc76188

Regenerate resource assertions

5e84438

Commit grant resource assertions

079f827

Regenerate resource parameter assertions

35365cf

Regenerate resource show output assertions

d14bc74

Regenerate show output schemas

fb8e053

Add image repository and organization account to the generator

a428541

Regenerate poc plugin framework model and schema builder

e8af89c

Remove additional imports from object parameters

b5c8fae

sfc-gh-asawicki added 6 commits September 19, 2025 13:56

Remove predefined imports logic

6051d16

Group const in gen mains

3007e06

Fix resource monitor show output schema

a9edcb1

Update go.mod

c2b7e2f

Remove spaces

d613308

Update the TODO

cbd72ef

sfc-gh-asawicki temporarily deployed to test September 19, 2025 11:59 — with GitHub Actions Inactive

sfc-gh-asawicki had a problem deploying to test September 19, 2025 12:15 — with GitHub Actions Failure

sfc-gh-asawicki marked this pull request as ready for review September 19, 2025 12:29

sfc-gh-asawicki requested a review from a team as a code owner September 19, 2025 12:29

sfc-gh-jcieslak approved these changes Sep 22, 2025

View reviewed changes

sfc-gh-jmichalak approved these changes Sep 22, 2025

View reviewed changes

sfc-gh-asawicki merged commit cf7dcd3 into dev Sep 22, 2025
19 of 20 checks passed

sfc-gh-asawicki deleted the chore-improve-imports-handling-in-generators branch September 22, 2025 13:09

sfc-gh-asawicki mentioned this pull request Sep 29, 2025

chore: Use common generator setup in SDK generator #4048

Merged

terraform-provider-release bot mentioned this pull request Oct 6, 2025

chore(main): release 2.8.0 #4066

Merged


		out, err := genhelpers.AddImports("", src)

		require.NoError(t, err)

		@@ -0,0 +1,11 @@
		{{- /gotype: github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/genhelpers.PreambleModel/ -}}

		// Code generated by {{ .GeneratorName }} generator (v{{ .GeneratorVersion }}); DO NOT EDIT.

chore: Improve imports handling in generators #4043

chore: Improve imports handling in generators #4043

Uh oh!

Conversation

sfc-gh-asawicki commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 19, 2025

Uh oh!

github-actions bot commented Sep 19, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sfc-gh-asawicki commented Sep 19, 2025 •

edited

Loading