-
Notifications
You must be signed in to change notification settings - Fork 555
feat: avm/res/cognitive-services/account support `modelProviderData…
#7199
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
Open
dmytroslotvinskyy
wants to merge
4
commits into
Azure:main
Choose a base branch
from
dmytroslotvinskyy:feat/cognitive-services-account-modelproviderdata
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
7a882e4
feat: `avm/res/cognitive-services/account` support `modelProviderData…
dmytroslotvinskyy 3c1d07e
Merge branch 'main' into feat/cognitive-services-account-modelprovide…
dmytroslotvinskyy ac3bff7
Merge branch 'main' into feat/cognitive-services-account-modelprovide…
dmytroslotvinskyy a7aba7c
Update avm/res/cognitive-services/account/CHANGELOG.md
dmytroslotvinskyy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This should be implemented as a child resource
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 @dmytroslotvinskyy since this was already implemented in this way we can keep it for this PR, but track the required update. Please create an issue for this resource module, to update the implementation of the deployments child resource as a separare child module. Please reply to this comment with the issue link. Thanks!
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.
Hello @eriqua
Created #7206 to track updating the deployments child resource as a separate child module. Thanks!
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.
Hi @eriqua,
I created #7206 to track the requested child-module refactor and prepared the implementation in #7207.
PR #7207 extracts
Microsoft.CognitiveServices/accounts/deploymentsinto a separatedeploymentchild module while preserving the existing parentdeploymentsinterface.Thanks!
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 @dmytroslotvinskyy, first thanks for working on this feedback that fast! Is #7207 intended to superseed this PR or should they be merged one after the other? Asking because if we merge this one first we'll need to update the other one rebasing to upstream and modifying for example the changelog accordingly. Please let me know and I'll proceed with the review based on that.
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.
Please help me on that ordering. I made proposal to make the tast easier. Appreciate your help! Additionally as you can see TF equipment has been made too