Skip to content

Conversation

@Mossaka
Copy link
Member

@Mossaka Mossaka commented Feb 25, 2021


This checklist is used to make sure that common guidelines for a pull request are followed.

General Guidelines

  • [] Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.

@yungezz
Copy link
Member

yungezz commented Feb 25, 2021

hi @fengzhou-msft is this the release for installation fixing? thanks

src/index.json Outdated
Comment on lines 10336 to 10347
"pyyaml (>=5.1.0)",
"azure-identity",
"msrest (>=0.6.18)",
"azure-core (<2.0.0,>=1.8.0)",
"azure-mgmt-core (<2.0.0,>=1.2.0)",
"marshmallow (<4.0.0,>=3.5)",
"tqdm",
"azure-storage-blob (<=12.5.0,>12.0.0b4)",
"pydash (<=4.9.0)",
"azure-storage-file-share (==12.3.0)",
"pathspec (==0.8.*)",
"cryptography (<=3.3.2)"
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove pyyaml, msrest, azure-core and azure-mgmt-core? They're already in the dependency list of azure-cli.

It is also recommended to replace azure-storage-blob and azure-storage-file-share with azure-multiapi-storage installed by azure-cli. You can refer to this PR: Azure/azure-cli#13414. @Juliehzl can help if you have any questions about storage dependency usages.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed these dependencies in index.json. Do I also need to remove them in setup.py? Will ping @Juliehzl for azure-storage-blob dependency.

Copy link
Member

Choose a reason for hiding this comment

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

You need to remove them in setup.py and regenerate the wheel and update index.json.

Copy link
Contributor

Choose a reason for hiding this comment

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

After discussed with @Mossaka, the storage sdk is required by azure-ml package, not cli extension. If there is no packaging issue, I am fine with it.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Juliehzl LMK if there is no packaging issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed dependencies in the CLI wheel @fengzhou-msft

@fengzhou-msft
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Mossaka
Copy link
Member Author

Mossaka commented Apr 7, 2021

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 3047 in repo Azure/azure-cli-extensions

@fengzhou-msft
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fengzhou-msft
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fengzhou-msft
Copy link
Member

Merge #3317 first before merging this PR.

@Mossaka Mossaka closed this May 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants