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

chore: sdk v2 implemented for password policy #1752

Merged
merged 18 commits into from
Apr 28, 2023
Merged

Conversation

sfc-gh-swinkler
Copy link
Collaborator

This is a new design for the SDK that supports constructing SQL queries as closely to the official documentation as possible, while still maintaining the appearance of a Rest API. Implemented for password policy as an example, more to follow suit.

pkg/sdk/password_policy_test.go Outdated Show resolved Hide resolved
pkg/sdk/client.go Outdated Show resolved Hide resolved
pkg/sdk/password_policy.go Outdated Show resolved Hide resolved
pkg/sdk/password_policy.go Outdated Show resolved Hide resolved
pkg/sdk/client.go Outdated Show resolved Hide resolved
pkg/resources/password_policy.go Outdated Show resolved Hide resolved
@github-actions
Copy link

Integration tests failure for 924924b6b4d543fc836984b89a7493a65180bfdc

pkg/sdk/type_helpers.go Outdated Show resolved Hide resolved
pkg/sdk/type_helpers.go Outdated Show resolved Hide resolved
pkg/sdk/type_helpers.go Outdated Show resolved Hide resolved
pkg/sdk/client.go Outdated Show resolved Hide resolved
@github-actions
Copy link

Integration tests failure for 588d00ec78eef7fd4537f08e2bd6ba7e076c5e54

@github-actions
Copy link

Integration tests failure for 2f21b3bc3537ec00c3b3a730a637fa737a596706

@github-actions
Copy link

Integration tests failure for d9223a8205216756dff865df8ebc9165f23babca

@github-actions
Copy link

Integration tests failure for 9252cfaa0dbf07322ec469a0f50bd17156ad5f71

@github-actions
Copy link

Integration tests failure for cddd13275e7cd53d7873fca33f1271ac1472bd53

@github-actions
Copy link

Integration tests success for e1894914801d8e8031a72b48ed43a15a02352433

pkg/sdk/object_type.go Outdated Show resolved Hide resolved
pkg/sdk/password_policy.go Show resolved Hide resolved
pkg/sdk/password_policy.go Show resolved Hide resolved
pkg/sdk/password_policy.go Show resolved Hide resolved
pkg/sdk/password_policy.go Outdated Show resolved Hide resolved
pkg/resources/password_policy.go Outdated Show resolved Hide resolved
pkg/resources/password_policy.go Outdated Show resolved Hide resolved
pkg/resources/password_policy.go Show resolved Hide resolved
pkg/resources/password_policy.go Outdated Show resolved Hide resolved
pkg/resources/password_policy.go Show resolved Hide resolved
pkg/sdk/client.go Outdated Show resolved Hide resolved
pkg/sdk/password_policy.go Outdated Show resolved Hide resolved
pkg/sdk/type_helpers.go Outdated Show resolved Hide resolved
@github-actions
Copy link

Integration tests failure for aad7afa5e20a1f1ef74c883f069af765614d50df

pkg/sdk/password_policy.go Outdated Show resolved Hide resolved
pkg/sdk/sql_builder.go Outdated Show resolved Hide resolved
pkg/sdk/sql_builder.go Outdated Show resolved Hide resolved
pkg/sdk/sql_builder.go Outdated Show resolved Hide resolved
pkg/sdk/helper_test.go Outdated Show resolved Hide resolved
pkg/sdk/helper_test.go Outdated Show resolved Hide resolved
pkg/sdk/password_policy_test.go Outdated Show resolved Hide resolved
pkg/sdk/password_policy_test.go Outdated Show resolved Hide resolved
pkg/sdk/helper_test.go Show resolved Hide resolved
pkg/sdk/helper_test.go Show resolved Hide resolved
pkg/sdk/helper_test.go Show resolved Hide resolved
@github-actions
Copy link

Integration tests failure for b8112b9627eb412ca2909ee6a1767348e9446a10

pkg/sdk/errors.go Outdated Show resolved Hide resolved
@github-actions
Copy link

Integration tests failure for e8acea5e785dbb583a68019d9826717b200d324d

@github-actions
Copy link

Integration tests failure for f1d60513a27fce578bd0eb9f54f0b57d3205ced8

@github-actions
Copy link

Integration tests failure for cd08827df47c7653c5b965f8d33b90bf86281fce

@github-actions
Copy link

Integration tests failure for 96996180eb0d26fae39acaad6ce0e0c71f1a58c0

pkg/sdk/client_test.go Outdated Show resolved Hide resolved
@github-actions
Copy link

Integration tests failure for 6673bfea3b832b5c39a663c51c15a586bf55b464

pkg/sdk/sql_builder.go Show resolved Hide resolved
pkg/sdk/sql_builder.go Outdated Show resolved Hide resolved
)

func (qt quoteType) Quote(v interface{}) string {
if qt == NoQuotes {
Copy link
Contributor

Choose a reason for hiding this comment

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

A switch statement on qt to handle all three cases separately would be nice.

assert.Equal(t, 0, len(passwordPolicies))
})

/* there appears to be a bug in the Snowflake API. LIMIT is not actually limiting the number of results
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you contacted the team in charge of this API?

pkg/sdk/password_policy.go Outdated Show resolved Hide resolved
Comment on lines +172 to +174
if count > 1 {
return errors.New("only one parameter can be unset at a time")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

Copy link
Contributor

Choose a reason for hiding this comment

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

This does not work in manual tests.

Comment on lines +152 to 154
if v, ok := d.GetOk("or_replace"); ok {
createOptions.OrReplace = sdk.Bool(v.(bool))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We can change Bool to be called like so::

Suggested change
if v, ok := d.GetOk("or_replace"); ok {
createOptions.OrReplace = sdk.Bool(v.(bool))
}
createOptions.OrReplace = sdk.Bool(d.GetOk("or_replace"))

with

func Bool(value interface{}, ok bool) *bool {
	if !ok {
		return nil
	}
	return &value.(bool)
}

Copy link
Contributor

Choose a reason for hiding this comment

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

we can put a function like this in the Terraform provider, but i don't want the SDK to have any context of the Terraform provider internals

Makefile Show resolved Hide resolved
@github-actions
Copy link

Integration tests failure for 6978cb4f9929af9ae05b2ec3614ee44a98e33b1b

@github-actions
Copy link

Integration tests success for 774c83363acd590aea6753aed6a9c104ebb7507c

@github-actions
Copy link

Integration tests success for f7e0af459f052cc4f97b0ef8d2459b28f56abe48

@github-actions
Copy link

Integration tests success for 2d5c9a344a801a806c570dc325edeeaae6e0e839

@sfc-gh-swinkler sfc-gh-swinkler merged commit 0cb1164 into main Apr 28, 2023
@sfc-gh-swinkler sfc-gh-swinkler deleted the client-rework2 branch April 28, 2023 22:53
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.

3 participants