Skip to content

Conversation

@LTA-Thinking
Copy link
Contributor

Regenerated the HealthcareAPIs cli extension to add support for export and managed identity.


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?
    I also ran the tests through azdev.

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.

@LTA-Thinking
Copy link
Contributor Author

Hello CLI team. I generated this with autorest and it introduced a number of style issues. Should I fix these manually, or is there a better way to fix them?

@yonzhan yonzhan added this to the S171 milestone May 26, 2020
@yonzhan
Copy link
Collaborator

yonzhan commented May 26, 2020

HealthcareAPIs

@haroldrandom
Copy link
Contributor

As for the style issue, we just run pylint and flake8 on your extension code, you could find the script in azure-cli-extensions/scripts/ci/source_code_static_analysis.py and run some of those checking step manually in your local machine.

As for the linter issue, there is a command healthcareapis service wait that missing help entry. you could run azdev linter --include-whl-extensions healthcareapis after azdev setup and azdev extension add healthcareapis.

BTW, why bother to re-generate this extension?

@LTA-Thinking
Copy link
Contributor Author

We added two new properties to our service that the CLI needs to support. We had the CLI generated for us when it was first created, so I used the new version of the autorest tool to regenerate the extension with the new properties. Is there another way to update an extension that is preferred? My understanding is that the CLI code is all automatically generated.

@haroldrandom
Copy link
Contributor

haroldrandom commented May 27, 2020

We added two new properties to our service that the CLI needs to support. We had the CLI generated for us when it was first created, so I used the new version of the autorest tool to regenerate the extension with the new properties. Is there another way to update an extension that is preferred? My understanding is that the CLI code is all automatically generated.

I see. It's really a problem that autorest need to take care, it doesn't consider how to upgrade. So everthing is brand new like new generated.

I suggest do not re-generate the extension from initial. We have an automatic release pipeline for developer to release their extension once the code is merged into master branch and your extension has upgraded to higher version. Downgrading version won't trigger the release unless human intervene manually.

So, I suggest we take care of this extension uprgade manually, pre-append changes to the history notes and upgrade the extension version.
It would be better to consider not to break user dramatically. :)

@LTA-Thinking
Copy link
Contributor Author

I reverted the changes to the history and added in a new version for these changes. I've also fixed the lint error for the wait command (we don't have one so I don't know why it was added).

@LTA-Thinking
Copy link
Contributor Author

Missed that the regeneration added the experimental flag. It is now removed.

@LTA-Thinking
Copy link
Contributor Author

@haroldrandom Is anything else required before merging the PR?

@haroldrandom
Copy link
Contributor

@fengzhou-msft could you please take a look this PR?

@LTA-Thinking
Copy link
Contributor Author

@fengzhou-msft?

@LTA-Thinking
Copy link
Contributor Author

@haroldrandom Is it possible to merge the changes? We have some people waiting on the update.

@haroldrandom haroldrandom changed the title Regenerate CLI for export and MI [healthcareapis] Regenerate CLI for export and MI Jun 11, 2020
@haroldrandom haroldrandom merged commit 894f73c into Azure:master Jun 11, 2020
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.

4 participants