-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Added Data Connectors Check Requirements to security insights #8020
Conversation
You don't have permission to trigger SDK Automation. |
Can one of the admins verify this patch? |
Hi @fengzhou-msft , I accidentally opened this PR from my personal Github account. I am a MS employee working in Azure Sentinel group. Thanks! |
/azp run automation – sdk |
No pipelines are associated with this pull request. |
@t-haorga please use your personal account which submitted this PR to sign CLA in the above comment. |
/openapi sdkautomation |
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
Azure Pipelines successfully started running 1 pipeline(s). |
azure-sdk-for-python - Release
|
azure-sdk-for-net - Release
|
azure-sdk-for-java - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
azure-sdk-for-go - Release
|
Commenter does not have sufficient privileges for PR 8020 in repo Azure/azure-rest-api-specs |
@fengzhou-msft signed |
@fengzhou-msft waiting for your approval |
I am adding some changes that we applied since I opened this PR. Please do not merge automatically yet but do review the PR. Thanks! |
"Check Data Connector Requirements" | ||
], | ||
"description": "Get requirements state for a data connector type.", | ||
"operationId": "DataConnectorRequirements_list", |
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.
DataConnectorRequirements_list [](start = 24, length = 30)
update
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.
Can you please explain why?
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.
it is a post operation.. _List is usually for collection.. probably rename the action name to '/listDataConnectorRequirements' ?
In reply to: 364130217 [](ancestors = 364130217)
"type": "object", | ||
"required": [ | ||
"kind" | ||
] |
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.
I suppose this is already being enforced at server level and even discriminator pattern may not work. hope that is the case.
otherwise, adding new required property is a breaking change.
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.
I added these because a test wouldn't pass, there was an error saying a discriminator must be required.
It is enforced in the server level as well.
What do you mean by "even discriminator pattern may not work" ?
@fengzhou-msft Please merge |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
"x-ms-enum": { | ||
"modelAsString": true, | ||
"name": "DataConnectorAuthorizationType", | ||
"values": [ |
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.
SDK automation tests failed because the values in x-ms-enum
do not match with enum
values above.
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.
Ok I fixed that now
/azp run automation - sdk |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run automation - sdk |
Commenter does not have sufficient privileges for PR 8020 in repo Azure/azure-rest-api-specs |
@fengzhou-msft I think it should be fine now, but I couldn't run the tests myself |
/azp run automation - sdk |
Azure Pipelines successfully started running 1 pipeline(s). |
@fengzhou-msft I think this check might be stuck - it is still waiting for status report after a few hours. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
@fengzhou-msft all tests passed, please merge. Thanks! |
Latest improvements:
MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.
Contribution checklist:
ARM API Review Checklist
Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.
Please follow the link to find more details on API review process.