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

feat: Add SDK for External Volumes #3033

Merged
merged 10 commits into from
Sep 5, 2024
Merged

Conversation

jdoldis
Copy link
Contributor

@jdoldis jdoldis commented Sep 2, 2024

This PR adds the SDK for External Volumes. This is part of a series of PRs aimed at adding provider support for Iceberg tables.

References

@jdoldis jdoldis marked this pull request as draft September 2, 2024 21:35
comment := "some comment"
id := createExternalVolume(
t,
append(append(append(append(s3StorageLocations, gcsStorageLocationsNoEncryption...), azureStorageLocations...), s3StorageLocationsNoEncryption...), gcsStorageLocations...),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once version 1.22 is available Slices.Concat can be used, which will clean these lines up a bit

client := testClient(t)
ctx := testContext(t)

awsBaseUrl := testenvs.GetOrSkipTest(t, testenvs.AwsExternalBucketUrl)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not exactly sure what these evaluate to on your side. The formats I was using locally -


TEST_SF_TF_AWS_EXTERNAL_BUCKET_URL=s3://my_example_bucket
TEST_SF_TF_AWS_EXTERNAL_KEY_ID=1234abcd-12ab-34cd-56ef-1234567890ab
TEST_SF_TF_AWS_EXTERNAL_ROLE_ARN=arn:aws:iam::123456789012:role/myrole
TEST_SF_TF_GCS_EXTERNAL_BUCKET_URL=gcs://my_example_bucket
TEST_SF_TF_AZURE_EXTERNAL_BUCKET_URL=azure://123456789.blob.core.windows.net/my_example_container
TEST_SF_TF_AZURE_EXTERNAL_TENANT_ID=123456789

@jdoldis jdoldis marked this pull request as ready for review September 3, 2024 07:01
Copy link
Collaborator

@sfc-gh-asawicki sfc-gh-asawicki left a comment

Choose a reason for hiding this comment

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

Good change!

I checked it and left mostly cosmetic comments. I also left a few pointers for the future contributions :)

Let's wait for @sfc-gh-jcieslak review too (it will happen tomorrow morning).

After that, I think we will run it to check the test results.

pkg/sdk/external_volumes_def.go Outdated Show resolved Hide resolved
pkg/sdk/external_volumes_def.go Outdated Show resolved Hide resolved
pkg/sdk/external_volumes_def.go Show resolved Hide resolved
pkg/sdk/external_volumes_def.go Show resolved Hide resolved
pkg/sdk/external_volumes_def.go Outdated Show resolved Hide resolved
pkg/sdk/external_volumes_gen_test.go Show resolved Hide resolved
pkg/sdk/testint/external_volumes_gen_integration_test.go Outdated Show resolved Hide resolved
@jdoldis
Copy link
Contributor Author

jdoldis commented Sep 3, 2024

Thanks for the review @sfc-gh-asawicki ! Have pushed changes for most of those suggestions and left a few comments on others.

Copy link
Collaborator

@sfc-gh-jcieslak sfc-gh-jcieslak left a comment

Choose a reason for hiding this comment

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

1 comment, otherwise 👍
Thanks for the contribution :)

pkg/sdk/external_volumes_impl_gen.go Outdated Show resolved Hide resolved
@jdoldis
Copy link
Contributor Author

jdoldis commented Sep 4, 2024

Thanks @sfc-gh-jcieslak , have updated that 🙂

@sfc-gh-asawicki
Copy link
Collaborator

/ok-to-test sha=20808a006e04687aef5c5744d4b07a2e6201169f

Copy link

github-actions bot commented Sep 4, 2024

Integration tests failure for 20808a006e04687aef5c5744d4b07a2e6201169f

@sfc-gh-asawicki sfc-gh-asawicki merged commit 2844a30 into Snowflake-Labs:main Sep 5, 2024
6 of 8 checks passed
sfc-gh-jmichalak pushed a commit that referenced this pull request Sep 19, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.96.0](v0.95.0...v0.96.0)
(2024-09-18)

Essential GA object readiness for V1:
[link](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/v1-preparations/ESSENTIAL_GA_OBJECTS.MD)
([Roadmap
reference](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/ROADMAP.md#preparing-essential-ga-objects-for-the-provider-v1)).

:exclamation: Migration guide: [v0.95.0 ->
v0.96.0](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#v0950--v0960)

### 🎉 **What's new:**

* V1 redesign of resources and data sources
* Row access policy
([#3066](#3066))
([#3063](#3063))
* Resource monitor
([#3052](#3052))
([#3064](#3064))
* Masking policy
([#3078](#3078))
([#3083](#3083))
* SDK upgrades
* External volume
([#3033](#3033))
* Authentication policy
([#2937](#2937))
([#3068](#3068))
([#3061](#3061))


### 🔧 **Misc**

* Clean up old test object helpers
([#3049](#3049))
* Add example of granting role to multiple objects
([#3047](#3047))
* Update readme and objects rework state
([#3046](#3046))

### 🐛 **Bug fixes:**

* Fix model grants
([#3070](#3070))
* Fix database show by and resource logic
([#3055](#3055))
* Fix default secondary roles option import
([#3041](#3041))
* Fix sweepers for warehouse and database
([#3057](#3057))
* Fix views permadiff
([#3079](#3079))
* Update v0.95.0 migration guide
([#3062](#3062))

Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>
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