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

Fix sequence import just for review, this wont be merged #3

Closed
wants to merge 25 commits into from

Conversation

jtzero
Copy link

@jtzero jtzero commented Aug 19, 2021

this is what will be pushed upstream

the release for our purposes is here #4

first #2

Test Plan

  • acceptance tests

References

@jtzero jtzero requested review from dfedde and jtrtj August 19, 2021 14:57
@jtzero jtzero force-pushed the fix-sequence-import branch from 39c817e to 13d8e39 Compare August 19, 2021 15:04
@jtzero jtzero force-pushed the fix-sequence-import branch from 13d8e39 to eafd113 Compare August 19, 2021 22:39
@jtrtj jtrtj force-pushed the fix-sequence-import branch 2 times, most recently from 6950621 to 78e07ca Compare August 20, 2021 22:44
@jtzero jtzero force-pushed the fix-sequence-import branch from ef1c0fd to d8533bc Compare August 20, 2021 22:54
@jtzero jtzero changed the title Fix sequence import Fix sequence import upstream Aug 23, 2021
@jtzero jtzero changed the title Fix sequence import upstream Fix sequence import just for review, this wont be merged Aug 23, 2021
@jtzero jtzero force-pushed the fix-sequence-import branch from d8533bc to 2962e6b Compare August 23, 2021 20:54
@github-actions
Copy link

Integration tests failure for 2962e6b0083e322b5c0872ed169ca43ae64fc566

@jtzero jtzero force-pushed the fix-sequence-import branch 2 times, most recently from 41050f7 to 656f775 Compare August 27, 2021 22:46
@@ -40,11 +41,13 @@ var sequenceSchema = map[string]*schema.Schema{
Type: schema.TypeString,
Required: true,
Description: "The database in which to create the sequence. Don't use the | character.",
ForceNew: true,
Copy link

Choose a reason for hiding this comment

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

these force news will cause the sequence count(next val) to reset when these are updated. is that expected/wanted?

Copy link
Author

Choose a reason for hiding this comment

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

yes for a new database or schema it follows the same path that other tf-snowflake resources follow

Copy link
Author

Choose a reason for hiding this comment

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

actually it won't reset it, but it will rebuild it with the original value

nextValue, err := strconv.Atoi(sequence.NextValue.String)
	if err != nil {
		return err
	}

sq.WithStart(nextValue)

@jtzero jtzero force-pushed the fix-sequence-import branch 4 times, most recently from db3350d to 937058f Compare December 1, 2021 19:47
@jtzero jtzero force-pushed the fix-sequence-import branch from 937058f to 3163818 Compare December 15, 2021 15:05
@jtzero jtzero force-pushed the fix-sequence-import branch from 3163818 to 97f515a Compare January 30, 2022 00:10
@jtzero jtzero force-pushed the fix-sequence-import branch from 97f515a to 75fe0d8 Compare January 30, 2022 03:57
@jtzero jtzero closed this Jan 30, 2022
@jtzero jtzero reopened this Jan 30, 2022
@jtzero jtzero closed this Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.