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

This package depends on [email protected] which doesn't play well with WebPack #8483

Closed
MTCMarkFranco opened this issue Apr 22, 2020 · 12 comments
Assignees
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. KeyVault

Comments

@MTCMarkFranco
Copy link

This package depends on [email protected] which doesn't play well with WebPack see related issue here: node-fetch/node-fetch#667

can we update this model to leverage version 3 of node-fetch to fix this problem. Getting "TypeError: Expected signal to be an instanceof AbortSignal" whenever we const client = new SecretClient(url, credential); after we receive DefaultAzureCredential() using MSI.

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Apr 22, 2020
@ghost ghost added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Apr 22, 2020
@ghost ghost added question The issue doesn't require a change to the product in order to be resolved. Most issues start as that and removed needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Apr 22, 2020
@xirzec xirzec added Azure.Core Cosmos pillar-performance The issue is related to performance, one of our core engineering pillars. Client This issue points to a problem in the data-plane of the library. labels Apr 23, 2020
@xirzec
Copy link
Member

xirzec commented Apr 23, 2020

The three packages which take this dependency are: cosmos, core-http, and perfstress.

@daviwil @southpolesteve any thoughts on updating this dep? I can take a stab at the actual PR, but wanted to make sure there wasn't anything blocking this work.

/cc @bterlson @ramya-rao-a

@bterlson
Copy link
Member

I'm not aware of anything blocking the upgrade fwiw.

@southpolesteve
Copy link
Contributor

@MTCMarkFranco Can you provide more info about exactly which libraries you are using and how you are using webpack? const client = new SecretClient(url, credential) isn't something that would be used with @azure/cosmos. We don't support any MSI auth with that package.

@xirzec @bterlson This needs more thorough consideration. node-fetch v3 is still in beta. It also has a significant breaking change that likely will break webpack+browser users

cc/ @zfoster

@bterlson
Copy link
Member

Since we aren't using node-fetch in the browser I hope that change won't affect us? We already map to an xhr implementation by default in browsers.

@southpolesteve
Copy link
Contributor

@azure/cosmos does use native fetch in browsers, but other packages may not. We rely on the node-fetch browser field point to be a simple wrapper around the native fetch

@southpolesteve
Copy link
Contributor

@southpolesteve
Copy link
Contributor

Removing the cosmos labels since I don't believe this is referring to the Cosmos Data Plane SDK

@daviwil
Copy link
Contributor

daviwil commented Jun 22, 2020

I agree with @southpolesteve about node-fetch 3.0, it's probably better to wait until there's a stable release before moving over to it.

@xirzec
Copy link
Member

xirzec commented Jun 23, 2020

The new pipeline doesn't depend on node-fetch, just node's https module: https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/core/core-https/src/nodeHttpsClient.ts

@ramya-rao-a ramya-rao-a added feature-request This issue requires a new behavior in the product in order be resolved. and removed question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Jul 16, 2020
@MTCMarkFranco
Copy link
Author

MTCMarkFranco commented Jul 24, 2020

@MTCMarkFranco Can you provide more info about exactly which libraries you are using and how you are using webpack? const client = new SecretClient(url, credential) isn't something that would be used with @azure/cosmos. We don't support any MSI auth with that package.

@xirzec @bterlson This needs more thorough consideration. node-fetch v3 is still in beta. It also has a significant breaking change that likely will break webpack+browser users

cc/ @zfoster

Hi There,

sorry for the late response...my environment details:

NodeJS Team MessageExtension generated by the Team Yo generator. Basically, I was incorporating KeyVault access into the project (not part of the default template) using the following libraries:

@azure/identity
@azure/keyvault-secrets

full code:
const credential = new DefaultAzureCredential();
const vaultName = process.env["KEYVAULT_NAME"];
const kvUrl =https://${vaultName}.vault.azure.net;
const client = new SecretClient(kvUrl, credential);

@xirzec xirzec added KeyVault and removed pillar-performance The issue is related to performance, one of our core engineering pillars. labels May 7, 2021
@xirzec
Copy link
Member

xirzec commented May 7, 2021

This should be resolved once we upgrade KeyVault to use CoreV2

@ramya-rao-a
Copy link
Contributor

Closing this issue in favor of #15594 that tracks the work needed to update all our packages that use core-http to now use the newer core packages.

@MTCMarkFranco, please subscribe to #15522 and #15595 that specifically tracks the work needed to move the Key Vault and Identity packages (the ones you are using) to use the new Azure Core packages.

cc @sadasant, @maorleger

@xirzec xirzec removed this from the Backlog milestone May 18, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. KeyVault
Projects
None yet
Development

No branches or pull requests

8 participants