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

Go SDK Helper Functions for Datalake #390

Merged
merged 59 commits into from
Oct 24, 2019

Conversation

clmccart
Copy link
Contributor

@clmccart clmccart commented Oct 18, 2019

What this PR does / why we need it:
This PR contains the azure go sdk helper functions for creating, getting, and deleting azure datalake enabled storage accounts as well as create, get, and delete for filesystems.
Additionally, this PR contains a shared access key authorizer to compensate for a gap in the azure autorest sdk. This authorizer was adapted from an open PR on the azure/go-autorest sdk.

closes #388
closes #329

Special notes for your reviewer:
I tried my best to isolate the autorest patch so that, once that PR gets merged in, it should be a pretty simple change to use the sdk instead of the files included in this PR. Would love feedback on the approach. The code cherry-picked from the outstanding PR on the autorest sdk is in pkg/resourcemanager/autorest

If applicable:

  • this PR contains documentation
  • this PR contains tests

@clmccart
Copy link
Contributor Author

In this PR, there is a adlsgen2 resource manager which is very similar to the storage account resource manager except for that it enforces a few things that are required for a storage account to be a datalake (hierachical namespace enabled and storage type). I was operating under the assumption that it might make sense to have adls be it's own operator that is similar to storage account but it sounds like this may not make sense. Let me know if I should remove the adlsgen2 resourcemanager and just work off of storage. If so, it may also make sense to move the filesystem files under the storages directory.

Copy link
Contributor

@jananivMS jananivMS left a comment

Choose a reason for hiding this comment

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

Had a few comments but otherwise looks good.

pkg/resourcemanager/iam/authorizers.go Outdated Show resolved Hide resolved
pkg/resourcemanager/storages/filesystem.go Outdated Show resolved Hide resolved
pkg/resourcemanager/storages/filesystem.go Outdated Show resolved Hide resolved

accountKey, err := getAccountKey(ctx, groupName, accountName, adlsClient)
if err != nil {
log.Fatalf("failed to get the account key for the authorizer: %v\n", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

To be consistent with other files, I would use log.Error or log.Info instead of Fatalf

Copy link
Contributor

@jananivMS jananivMS left a comment

Choose a reason for hiding this comment

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

I would recommend chaning to log.Error instead of fatalf

@clmccart clmccart merged commit e8c01e4 into Azure:azure-datalake Oct 24, 2019
@clmccart clmccart deleted the adls/gosdk_helpers branch October 28, 2019 17:18
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.

2 participants