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 explicit requirement for azure-core in mgmt-compute #34808

Closed
wants to merge 1 commit into from

Conversation

psalaberria002
Copy link

#34770 added a direct dependency for azure.core, and it actually requires this to be newer than 1.28.
Related to #34091

Description

Please add an informative description that covers that changes made by the pull request and link all relevant issues.

If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

Azure#34770 added a direct dependency for azure.core, and it actually requires this to be newer than 1.28. 
 Related to Azure#34091
@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 Mar 18, 2024
Copy link

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

Copy link

@jim-cooley jim-cooley left a comment

Choose a reason for hiding this comment

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

though i can't approve, this indeed fixes the issue with azure-mgmt-compute requirement of azure-core. specifically, azure-mgmt-compute requires the property 'SensitiveHeaderCleanupPolicy' on 'azure.core.pipeline.policies', which is only present in versions of azure-core >= 1.28.1

@annatisch
Copy link
Member

Hi @msyyc - could you confirm whether this setup.py is auto-generated and therefore the codegen needs updating for this dependency?

@msyyc
Copy link
Member

msyyc commented Mar 22, 2024

Hi @psalaberria002

Above all, if your application want to use SensitiveHeaderCleanupPolicy, it is better to upgrade dependency by yourself instead of changing SDK dependency. After all, not all SDK users need SensitiveHeaderCleanupPolicy

@msyyc msyyc closed this Mar 22, 2024
@jim-cooley
Copy link

jim-cooley commented Mar 22, 2024

actually, i don't want to use SensitiveHeaderCleanupPolicy, but if you write the following code:
self.credential = DefaultAzureCredential()
self.resource_client = ResourceManagementClient(self.credential, self.subscription_id)
self.compute_management_client = ComputeManagementClient(self.credential, self.subscription_id)

It will fail with "azure.core.pipeline.policies" does not have property 'SensitiveHeaderCleanupPolicy'. that is when using the latest mgmt-compute with an azure-core prior to 1.28.1. perhaps the reference is indirect via azure-mgmt-core, however the SDK itself uses this property so this fails at runtime.

I believe the error is here:

policies.SensitiveHeaderCleanupPolicy(**kwargs) if self._config.redirect_policy else None,
and not in the default policies you referred to. If there exists a redirect_policy, then it will attempt the SensitiveHeaderCleanupPolicy -- whether or not that is the redirect policy. But i will need to regenerate the error to give you a proper stack trace -- and if you reopen this issue I will do that for you -- it was indeed in the client or azure-core and related to an 'if self._config.redirect_policy else None' clause. here is the other location that springs up:
SensitiveHeaderCleanupPolicy(**kwargs) if config.redirect_policy else None,

@jim-cooley
Copy link

jim-cooley commented Mar 22, 2024

/reopen?

@aszodiba
Copy link

aszodiba commented Apr 3, 2024

I'm having totally the same issue as it is described by @jim-cooley.
I'll try to simply access synapse notebook contents by using azure-synapse-artifacts.
Using the recommended ArtifactsClient by simply adding credentials and the endpoint I get the same error. Hence it should be reopened.
image

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.

5 participants