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

Azure Redis .Net SDK update for new api v2023-08-01 #39230

Merged
merged 23 commits into from
Dec 15, 2023

Conversation

koderjoker
Copy link
Member

Contributing to the Azure SDK

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

Swagger: https://github.com/Azure/azure-rest-api-specs/blob/main/specification/redis/resource-manager/Microsoft.Cache/stable/2023-08-01/redis.json

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

@github-actions github-actions bot added Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Oct 12, 2023
@github-actions
Copy link

Thank you for your contribution @koderjoker! We will review the pull request and get back to you soon.

@pallavit
Copy link
Contributor

@koderjoker this PR is getting tagged as a community contribution. Please review the Azure SDK onboarding documentation and ensure that your GitHub organization membership and permissions are correct.

Azure SDK onboarding (Microsoft internal)
Azure SDK onboarding assistance (Microsoft internal)

/cc: @ArthurMa1978. Also please ensure you have the correct aliases are added to the code owners file - https://github.com/Azure/azure-sdk-for-net/blob/main/.github/CODEOWNERS

@koderjoker
Copy link
Member Author

koderjoker commented Oct 13, 2023

@pallavit submitted request for Azure SDK Partners. No new names to add to Code Owners from our side

@azure-sdk
Copy link
Collaborator

azure-sdk commented Oct 13, 2023

API change check

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

Azure.ResourceManager.Redis

austintolani and others added 7 commits October 18, 2023 09:22
- Adds test resource json file
- Creates FTs for new flush operation
- Updates assets.json after pushing new session records
Adds functional tests for aad
@koderjoker
Copy link
Member Author

koderjoker commented Nov 21, 2023

Test recording for all tests, for new API version in progress, will push session records and update assets.json once complete

* Update Channel Tests

* Update correct data type
@koderjoker
Copy link
Member Author

koderjoker commented Nov 29, 2023

Copy link
Member

@jsquire jsquire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@koderjoker: You are still coming up as having none of the public organizations required nor write permissions in the repository. Please review the Azure SDK onboarding guide.
This will need to be corrected before this PR can move forward.

@koderjoker
Copy link
Member Author

@jsquire I'm already a part of Azure GitHub and Azure SDK partners. Applied for write permissions for Azure cache for Redis SDKs.

@jsquire
Copy link
Member

jsquire commented Nov 30, 2023

@jsquire I'm already a part of Azure GitHub and Azure SDK partners. Applied for write permissions for Azure cache for Redis SDKs.

These organization memberships must be public on your profile and you must have write permissions to the Azure SDK repository. Your memberships are private and you lack write permissions. Please refer to the onboarding guide for further context.

@Azure Azure deleted a comment from azure-pipelines bot Dec 2, 2023
@Azure Azure deleted a comment from azure-pipelines bot Dec 2, 2023
@Azure Azure deleted a comment from azure-pipelines bot Dec 2, 2023
@koderjoker
Copy link
Member Author

@jsquire thanks, I've ensured that I have the correct permissions now

@koderjoker
Copy link
Member Author

Hi @archerzz @jsquire @tg-msft @KrzysztofCwalina @ArcturusZhang @pallavit @ArthurMa1978 @live1206! Who would be the assigned reviewer for this PR?

Despite the PR having the latest code generated and recordings for 2023-08-01 https://github.com/Azure/azure-sdk-assets/releases/tag/net%2Fredis%2FAzure.ResourceManager.Redis_50c384bcc2, the pipeline is showing errors regarding both

@archerzz
Copy link
Member

archerzz commented Dec 4, 2023

Hi @archerzz @jsquire @tg-msft @KrzysztofCwalina @ArcturusZhang @pallavit @ArthurMa1978 @live1206! Who would be the assigned reviewer for this PR?

Despite the PR having the latest code generated and recordings for 2023-08-01 Azure/azure-sdk-assets@net%2Fredis%2FAzure.ResourceManager.Redis_50c384bcc2 (release), the pipeline is showing errors regarding both

@koderjoker I think the error message is quite clear:

Generated code is not up to date.
    You may need to rebase on the latest main, 
    run 'eng\scripts\Update-Snippets.ps1' if you modified sample snippets or other *.md files (https://github.com/Azure/azure-sdk-for-net/blob/main/CONTRIBUTING.md#updating-sample-snippets), 
    run 'eng\scripts\Export-API.ps1' if you changed public APIs (https://github.com/Azure/azure-sdk-for-net/blob/main/CONTRIBUTING.md#public-api-additions). 
    run 'dotnet build /t:GenerateCode' to update the generated code and samples.
    
To reproduce this error locally, run 'eng\scripts\CodeChecks.ps1 -ServiceDirectory redis'.

@archerzz
Copy link
Member

archerzz commented Dec 4, 2023

Besides, looks like you have some test recordings which need update.

@koderjoker
Copy link
Member Author

koderjoker commented Dec 4, 2023

Hi @archerzz @jsquire @tg-msft @KrzysztofCwalina @ArcturusZhang @pallavit @ArthurMa1978 @live1206! Who would be the assigned reviewer for this PR?
Despite the PR having the latest code generated and recordings for 2023-08-01 Azure/azure-sdk-assets@net%2Fredis%2FAzure.ResourceManager.Redis_50c384bcc2 (release), the pipeline is showing errors regarding both

@koderjoker I think the error message is quite clear:

Generated code is not up to date.
    You may need to rebase on the latest main, 
    run 'eng\scripts\Update-Snippets.ps1' if you modified sample snippets or other *.md files (https://github.com/Azure/azure-sdk-for-net/blob/main/CONTRIBUTING.md#updating-sample-snippets), 
    run 'eng\scripts\Export-API.ps1' if you changed public APIs (https://github.com/Azure/azure-sdk-for-net/blob/main/CONTRIBUTING.md#public-api-additions). 
    run 'dotnet build /t:GenerateCode' to update the generated code and samples.
    
To reproduce this error locally, run 'eng\scripts\CodeChecks.ps1 -ServiceDirectory redis'.

@archerzz running eng\scripts\CodeChecks.ps1 -ServiceDirectory redis as mentioned in the error, on our local gives us 0 errors.

No new code is generated when eng\scripts\Export-API.ps1 or dotnet build /t:GenerateCode are run, and no snippets on running eng\scripts\Update-Snippets.ps1

Are there any steps we could take to see what is left to be generated, and unblock ourselves on this issue?

@koderjoker
Copy link
Member Author

koderjoker commented Dec 4, 2023

Besides, looks like you have some test recordings which need update.

Test recordings have already been recorded for this api version, for all the older and new tests (2023-08-01) https://github.com/Azure/azure-sdk-assets/tree/net/redis/Azure.ResourceManager.Redis_50c384bcc2/net/sdk/redis/Azure.ResourceManager.Redis/tests/SessionRecords, and assets.json updated.

Unable to find a record for the request PUT https://management.azure.com/subscriptions/3f43c5ac-2f70-4e2c-87ff-9a6822cbc0dc/resourcegroups/testRG-1352?api-version=2022-09-01
And this resource group request uri doesn't exist in any of our recordings. Is it not picking up the latest recordings? All tests are failing with this error.

Have we missed out on configuring or uploading any file, to cause this issue?

@jsquire
Copy link
Member

jsquire commented Dec 4, 2023

@jsquire thanks, I've ensured that I have the correct permissions now

You still do not have a public Microsoft organization membership. Please correct.

@koderjoker
Copy link
Member Author

koderjoker commented Dec 5, 2023

@jsquire corrected as requested, sorry for the confusion

@jsquire
Copy link
Member

jsquire commented Dec 5, 2023

koderjoker

Thanks. Confirmed that we're looking good. Removing my block. Please work with Arthur and team on the code review.

@jsquire jsquire dismissed their stale review December 5, 2023 15:24

Issues have been corrected.

@koderjoker koderjoker requested a review from archerzz December 13, 2023 11:13
Copy link
Member

@archerzz archerzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@koderjoker LGTM.

By the way, if you are going to release a new version, don't forget to update CHANGELOG.md. See here for details.

@archerzz archerzz merged commit da78e04 into Azure:main Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants