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

Make the parent of a Tenant Resource to be OperationsBase #1392

Closed

Conversation

fengzhou-msft
Copy link
Member

@fengzhou-msft fengzhou-msft commented Jul 9, 2021

Description

A container of tenant resource whose direct parent is tenant (for instance, the billing account resource in TenantOnly tests) should be able to get created with TenantOperations which is of type OperationsBase instead of ResourceOperationsBase.

This PR makes corresponding changes to support that and relies on core changes Azure/azure-sdk-for-net#22589

Todo:

  1. Add TenantOperationsExtensionWriter.
  2. In ListAsGenericResourceAsync(), Parent is cast as ResourceGroupOperations. This does not apply for resources whose ancestor is subscription or tenant (here a subscription ancestor means subscription has a direct child resource that is not a resource group, that child resource and the children of that child resource, their ancestor is subscription. A tenant ancestor is similar. Each resource's ancestor is either a resource group, a subscription or a tenant).

Checklist

To ensure a quick review and merge, please ensure:

  • The PR has a understandable title and description explaining the why and what.
  • The PR is opened in draft if not ready for review yet.
    • If opened in draft, please allocate sufficient time (24 hours) after moving out of draft for review
  • The branch is recent enough to not have merge conflicts upon creation.

Ready to Land?

  • Build is completely green
    • Submissions with test failures require tracking issue and approval of a CODEOWNER
  • At least one +1 review by a CODEOWNER
  • All -1 reviews are confirmed resolved by the reviewer
    • Override/Marking reviews stale must be discussed with CODEOWNERS first

@@ -121,7 +121,7 @@ public AvailabilitySetsCreateOrUpdateOperation StartCreateOrUpdate(string availa
}

var originalResponse = _restClient.CreateOrUpdate(Id.ResourceGroupName, availabilitySetName, parameters, cancellationToken: cancellationToken);
return new AvailabilitySetsCreateOrUpdateOperation(Parent, originalResponse);
return new AvailabilitySetsCreateOrUpdateOperation(this, originalResponse);
Copy link
Member

@ArcturusZhang ArcturusZhang Jul 10, 2021

Choose a reason for hiding this comment

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

Why is this needed? This looks like quite a big difference

Copy link
Member Author

@fengzhou-msft fengzhou-msft Jul 10, 2021

Choose a reason for hiding this comment

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

Originally, I wanted to avoid using the Parent as much as possible as ResourceContainerBase has a constructor that does not set Parent, so Parent could be null. But it will cause other issues to use this here. I reverted the change.

@fengzhou-msft fengzhou-msft changed the title constructors with OperationsBase Make the parent of a Tenant Resource to be OpreationsBase Jul 10, 2021
@fengzhou-msft fengzhou-msft changed the title Make the parent of a Tenant Resource to be OpreationsBase Make the parent of a Tenant Resource to be OperationsBase Jul 10, 2021
Co-authored-by: Arcturus <ufo54153@gmail.com>
Comment on lines +14 to +15
<!-- <PackageReference Include="Azure.ResourceManager.Core" Version="1.0.0-alpha.20210706.2" /> -->
<ProjectReference Include="../../../../azure-sdk-for-net/sdk/resourcemanager/Azure.ResourceManager.Core/src/Azure.ResourceManager.Core.csproj" />
Copy link
Member Author

Choose a reason for hiding this comment

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

After Azure/azure-sdk-for-net#22589 is merged and published, use the new version of core.

@m-nash
Copy link
Member

m-nash commented Jul 13, 2021

/check-enforcer evaluate

@m-nash m-nash closed this Jul 14, 2021
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.

3 participants