-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[#3319] datalakestore gen2 filesystem resource #4457
Conversation
Hi @tombuildsstuff , could you please review and let me know if any more changes required, Thanks |
30a4dd3
to
9ff238d
Compare
@tombuildsstuff the travis tests are failing due to vendor file missing, should I enforce to add track of the filesystems on it? |
@tcz001 yep - if you don't mind I'll push a commit to fix that so that we can work through & review this :) |
@tombuildsstuff Thanks, I've fixed the lintrest issues, should be ready for review |
507d07f
to
c909e4d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @tcz001
Thanks for this PR :)
Taking a look through whilst this is off to a good start, after some discussions internally yesterday we need to take a slightly different direction for the Storage Resources going forward, primarily by using the Storage Account ID rather than the Storage Account Name to allow us to reduce some API calls.
Since we don't have any examples of this in the repository - I hope you don't mind but I'm going to push a couple of commits to both fix the comments I've left in-line - and update this to using an ID for the Storage Account, rather than the Name.
Thanks!
…ather than the name of the storage account & fixing PR comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a minor question
This has been released in version 1.35.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example: provider "azurerm" {
version = "~> 1.35.0"
}
# ... other configuration ... |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
This PR implements new resource of datalakestore gen2 filesystem, which requires giovanni recent support for adls gen2
This change should close #3319 feature request, I'm working on another PR to support data_source, will come back soon.