chore: Generate marshal json for each model #3307
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Integration tests failure for f2dc3bf06899bba9b305adf39807142f05cc9086 |
2 tasks
sfc-gh-jmichalak
approved these changes
Dec 20, 2024
| func ProcedureJavascriptBasicInline( | ||
| resourceName string, | ||
| id sdk.SchemaObjectIdentifierWithArguments, | ||
| returnType datatypes.DataType, | ||
| procedureDefinition string, | ||
| ) *ProcedureJavascriptModel { | ||
| return ProcedureJavascript(resourceName, id.DatabaseName(), id.Name(), procedureDefinition, returnType.ToSql(), id.SchemaName()) | ||
| return ProcedureJavascript(resourceName, id.DatabaseName(), id.Name(), procedureDefinition, returnType.ToSql(), id.SchemaName()). |
There was a problem hiding this comment.
procedureDefinition is passed twice in this whole call, and it's a bit confusing.
There was a problem hiding this comment.
This is on purpose now.
I added to the PR description the next step which explains it.
| func ProcedureSqlBasicInline( | ||
| resourceName string, | ||
| id sdk.SchemaObjectIdentifierWithArguments, | ||
| returnType datatypes.DataType, | ||
| procedureDefinition string, | ||
| ) *ProcedureSqlModel { | ||
| return ProcedureSql(resourceName, id.DatabaseName(), id.Name(), procedureDefinition, returnType.ToSql(), id.SchemaName()) | ||
| return ProcedureSql(resourceName, id.DatabaseName(), id.Name(), procedureDefinition, returnType.ToSql(), id.SchemaName()). |
| @@ -35,6 +35,9 @@ resource "snowflake_share" "test" { | |||
| Item{IntField: 2, StringField: "second item"}, | |||
| ). | |||
| WithSingleObject("one", 2). | |||
| WithTextFieldExplicitNull(). | |||
| WithListFieldEmpty(). | |||
| WithMultilineField("some\nmultiline\ncontent"). | |||
There was a problem hiding this comment.
Nit: test the behavior of \t.
There was a problem hiding this comment.
I'll do it in one of the next PRs
|
Integration tests failure for 4a36db81894f99c88b02a5da5b2acaf4d4f6eee6 |
|
Integration tests cancelled for 18a59381fdb40c2df9b4d7ecf67f969f333a019e |
sfc-gh-jmichalak
approved these changes
Dec 20, 2024
sfc-gh-asawicki
added a commit
that referenced
this pull request
Dec 20, 2024
- generate marshal and `depends_on` for each resource and data source model - replace deprecated `FromModel` with the new one utilizing the aforementioned marshaling - fix test with explicit empty list (and add it to special variables) - fix tests with explicit null value (and add it to special variables) - generalize multiline handling and test it - use multiline strings for definitions in functions and procedures config builders - simplify function and procedure definitions - format whitespace in function and procedure definitions For the next PRs (added to the issue): - current config building for functions/procedures (especially definitions) is complex: - definition is build using multiline golang strong and sprintf - whitespace is formatted - this is added in the config builder wrapped in special multiline marker - hcl-compatible json is generated - json converted to hcl - hcl formatted using custom formatters This should be refined and made simpler (e.g. newline replacement, encoding problems, etc.) - marking in config builders generators which fields should be handled as multiline by default (because now e.g. SQL function requires the definition, so it's generated in basic builder as a string input, and not the multiline one; because of that it has to be added again with `WithFunctionDefinitionValue(...)` which is confusing)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
depends_onfor each resource and data source modelFromModelwith the new one utilizing the aforementioned marshalingFor the next PRs (added to the issue):
current config building for functions/procedures (especially definitions) is complex:
This should be refined and made simpler (e.g. newline replacement, encoding problems, etc.)
marking in config builders generators which fields should be handled as multiline by default (because now e.g. SQL function requires the definition, so it's generated in basic builder as a string input, and not the multiline one; because of that it has to be added again with
WithFunctionDefinitionValue(...)which is confusing)