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

Add sdk for Content Safety #34933

Merged
merged 29 commits into from
May 26, 2023
Merged

Add sdk for Content Safety #34933

merged 29 commits into from
May 26, 2023

Conversation

mengaims
Copy link
Contributor

Contributing to the Azure SDK

Please see our CONTRIBUTING.md if you are not familiar with contributing to this repository or have questions.

For specific information about pull request etiquette and best practices, see this section.

@mengaims mengaims marked this pull request as draft March 15, 2023 10:05
@mengaims mengaims changed the title Add sdk for Project CarnegieCarnegie Add sdk for Content Safety Mar 17, 2023
@mengaims mengaims marked this pull request as ready for review April 26, 2023 13:29
@pallavit
Copy link
Contributor

@mengaims could you please update the codeowners file. We always recommend putting 2 owners in the list - https://github.com/Azure/azure-sdk-for-net/blob/main/.github/CODEOWNERS

@pallavit
Copy link
Contributor

/cc: @heaths for review/service buddy.

@pallavit pallavit requested a review from heaths April 26, 2023 17:47
@mengaims mengaims requested a review from ronniegeraghty as a code owner May 4, 2023 12:43
@mengaims
Copy link
Contributor Author

mengaims commented May 4, 2023

@mengaims could you please update the codeowners file. We always recommend putting 2 owners in the list - https://github.com/Azure/azure-sdk-for-net/blob/main/.github/CODEOWNERS

@pallavit Updated, please check the latest code in PR. Btw, do I need any other changes like changing the name of PR?

@mengaims
Copy link
Contributor Author

mengaims commented May 4, 2023

@heaths I've already updated following content in this PR:

  1. Tests and test recordings
  2. Samples
  3. Changelog
  4. Ci.yml

Could you help have one round of review of them and trigger the CI pipeline run?

And here are the items WIP:

  1. Readme
  2. Operations generation issue in Azure.AI.ContentSafety.netstandard2.0.cs (We could have an online discussion if you could attend the sync I sent)

Please let me know if I have other things missing.

@heaths
Copy link
Member

heaths commented May 4, 2023

Could you help have one round of review of them and trigger the CI pipeline run?

@m-nash should the onboarding tool be creating the pipelines automatically? @mengaims there's no pipeline for me to approve, but one was not created which seems the onboarding wizard should've taken care of. Or was there a step skipped in documentation? Looking through our wiki docs, it seems this should've been created automatically. I'll ping a few people offline for clarification.

@heaths
Copy link
Member

heaths commented May 4, 2023

/azp run prepare-pipelines

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@m-nash
Copy link
Member

m-nash commented May 4, 2023

/azp run net - contentsafety - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@m-nash
Copy link
Member

m-nash commented May 4, 2023

Now that the CI is hooked in it will start to report all the errors.

One thing I see is we are missing the tsp-location.yaml file which was used to do the dotnet build /t:GenerateCode. The CI will complain about this.

@mengaims
Copy link
Contributor Author

mengaims commented May 5, 2023

Now that the CI is hooked in it will start to report all the errors.

One thing I see is we are missing the tsp-location.yaml file which was used to do the dotnet build /t:GenerateCode. The CI will complain about this.

@m-nash , thanks for the reminding! I have a question here, the CI will only check the existence of this file or will try run dotnet build /t:GenerateCode according to it? Since I've met some errors when using remote branch in this file, after mitigate with referencing to local repository, it works.

The error is:
Copy-Item: D:\src\mengaims\azure-sdk-for-net-test\eng\common\scripts\TypeSpec-Project-Sync.ps1:23
Line |
23 | Copy-Item -Path $source -Destination $dest -Recurse -Force
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| Cannot find path
| 'D:\src\mengaims\sparse-spec\Azure.AI.ContentSafety\specification\cognitiveservices\ContentSafety' because it

0m
| does not exist.

@heaths
Copy link
Member

heaths commented May 5, 2023

@mengaims you need to run dotnet build /t:GenerateCode locally. See https://aka.ms/azsdk/dpcodegen/net. You also need to check in all the files including the client tsp.

@mengaims
Copy link
Contributor Author

mengaims commented May 5, 2023

@mengaims you need to run dotnet build /t:GenerateCode locally. See https://aka.ms/azsdk/dpcodegen/net. You also need to check in all the files including the client tsp.

Yes I've ran it locally.

m-nash added 3 commits May 10, 2023 22:00
comment out unused snippets
update test to pass base64 string to binarydata
add tsp-location and gen library
@azure-sdk
Copy link
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

Azure.AI.ContentSafety

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.

5 participants