Support static website Read() from id in addition to state to enable import#4035
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4035 +/- ##
==========================================
- Coverage 57.86% 57.85% -0.01%
==========================================
Files 83 83
Lines 13375 13391 +16
==========================================
+ Hits 7739 7747 +8
- Misses 5037 5045 +8
Partials 599 599 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
danielrbradley
left a comment
danielrbradley
left a comment
There was a problem hiding this comment.
This looks okay to me .. I assume this was only a bug in the azidentity version?
| func (r *staticWebsite_azidentity) newStorageAccountClientForAccount(accName string) (*azblob.Client, error) { | ||
| return newStorageAccountClientForAccount(accName, r.env, r.creds) | ||
| } |
There was a problem hiding this comment.
This new method threw me a little when reading the diff being the same name as the thing it calls within. Looks like it's only called in one place, could we just call inner function directly there?
There was a problem hiding this comment.
This pattern mirrors the existing newStorageAccountClient that also exists twice. I think it makes sense - they do the same thing but the method on the resource needs fewer arguments.
It's a bug in both but I don't think we should spend time fixing the autorest code anymore. I'm hoping to remove it soon. |
thomas11
force-pushed
the
tkappler/staticwebsite-import
branch
from
1d69316 to
ba98978
Compare
|
This PR has been shipped in release v2.89.2. |
3 participants
When importing, there is no state yet and
Readis called with only the id. That's enough but the previous code didn't expect that and failed.Resolves #4028.