Skip to content

Conversation

@haroldrandom
Copy link
Contributor

Description
As this PR #12367 pointing out, some user may manually install extension from source code and still want to load it.

Same scenario could happen in the developing of azure-cli-ml as discussed offline. The develop CLI extension without azdev, and use that way to work in editable mode in older CLI (<= 2.4.0)

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change.
[Component Name 2] az command b: Add some customer-facing feature.


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

@haroldrandom haroldrandom added the Extensions `az extension` commands or extension infrastructure label May 1, 2020
@haroldrandom haroldrandom added this to the S170 milestone May 1, 2020
@haroldrandom haroldrandom self-assigned this May 1, 2020
@yonzhan yonzhan requested review from fengzhou-msft, jiasli and jsntcy May 1, 2020 05:00
@yonzhan
Copy link
Collaborator

yonzhan commented May 1, 2020

add to S170

Comment on lines +42 to +43
self.assertTrue(metadata['azext.isPreview'])
self.assertTrue(metadata['azext.isExperimental'])
Copy link
Member

Choose a reason for hiding this comment

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

I remember these two tags are not supposed to be both True in the future. @zhoxing-ms for confirmation.

Copy link
Contributor

@zhoxing-ms zhoxing-ms May 6, 2020

Choose a reason for hiding this comment

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

Yes, according to the previous design of @jiasli for isExperimental, an error will be reported if both isPreview and isExperimental are True. @jiasli Please help to see if my understanding is correct~

Copy link
Contributor Author

@haroldrandom haroldrandom May 7, 2020

Choose a reason for hiding this comment

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

Thanks for sharing this, I just learnt that.
I think the class WheelExtension and DevExtension is to represent data model not validation. Validation should be a higher level business.
This PR is about to test reading the metadata correctly, not validation for isExperimental and isPreview unless we decide to make validation here.

Please @jiasli make sure that the concern above has been made in CLI.


ext_name, ext_version = 'hello', '0.1.0'

ext_extension = WheelExtension(ext_name, os.path.join(self.ext_dir, ext_name + '-' + ext_version))
Copy link
Member

Choose a reason for hiding this comment

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

not use DevExtension?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to test WheelExtension class not DevExtension.
The user using this way to develop Azure CLI extension is to hack the machenism. I think it's doable but not recommended. And supporting .egg-info is just one month ago.

@yonzhan yonzhan requested a review from zhoxing-ms May 6, 2020 13:29
@haroldrandom haroldrandom marked this pull request as ready for review May 7, 2020 04:33
@haroldrandom haroldrandom merged commit a6fc7da into Azure:dev May 7, 2020
@haroldrandom haroldrandom deleted the editable-whl-type-support-in-developing-extension branch May 7, 2020 07:09
@haroldrandom haroldrandom modified the milestones: S170, S169 - For Build May 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Extensions `az extension` commands or extension infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants