Skip to content

Conversation

@calvinsID
Copy link
Contributor

@calvinsID calvinsID commented Apr 22, 2021

Description

image

image

image

Testing Guide
This is not exposed in a command yet, as I'm not sure what command to put it under. We will be using this function in webapp commands in the future, and static webapps have use cases as well


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

@calvinsID calvinsID requested a review from panchagnula April 22, 2021 19:01
@calvinsID calvinsID force-pushed the github-access-token branch from ace0a06 to 788d2d0 Compare April 22, 2021 19:04
@calvinsID calvinsID changed the title Add function to retrieve users github personal access token [AppService] Add function to retrieve users github personal access token Apr 22, 2021
@calvinsID calvinsID added this to the S186 milestone Apr 22, 2021
@yungezz yungezz added the App Services az appservice label Apr 23, 2021
Copy link
Member

Choose a reason for hiding this comment

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

Could you share where this ID comes from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's an oauth app created by AppService CLI team: https://github.com/AzureAppServiceCLI
Is there an official Azure oauth app we can use, or should we stick to this one?

@calvinsID calvinsID changed the title [AppService] Add function to retrieve users github personal access token [AppService] az appservice: Add function to retrieve users github personal access token Apr 23, 2021
@calvinsID calvinsID force-pushed the github-access-token branch from 9dfc04d to e6c8ae4 Compare April 23, 2021 18:45
Copy link
Contributor

@panchagnula panchagnula left a comment

Choose a reason for hiding this comment

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

Can't wait for supporting GitHub actions on CLI with full GitHub authentication with this. Thank you.

@qwordy
Copy link
Member

qwordy commented Apr 26, 2021

"Azure.azure-cli (Check CLI Style)" has problems. We are fixing it. New version of astroid changed __version__ to version.

@calvinsID calvinsID force-pushed the github-access-token branch from b6e1450 to 6c1e39e Compare April 26, 2021 19:01
@calvinsID
Copy link
Contributor Author

calvinsID commented Apr 26, 2021

Hi @qwordy do you mind re-reviewing the latest commit? I added additional changes to support az webapp deployment github-actions command in the webapp extensions repo: Azure/azure-cli-extensions#3303

@panchagnula
Copy link
Contributor

Azure/azure-cli-extensions#3303

@calvinsID can you update the title to include the support for GitHub actions as well since this will be used automatically to update the release history documentation to the users?

@calvinsID calvinsID changed the title [AppService] az appservice: Add function to retrieve users github personal access token [AppService] az appservice: Add function to retrieve users github personal access token, add pieces for Github Actions support Apr 27, 2021
Copy link
Member

@qwordy qwordy Apr 29, 2021

Choose a reason for hiding this comment

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

Also add it to setup.py otherwise pip install will fail

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added. Why are only some of the requirements.txt added to setup.py dependencies?

Copy link
Member

@qwordy qwordy Apr 30, 2021

Choose a reason for hiding this comment

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

pip install reads setup.py. Windows MSI artifact build reads requirements.txt. It makes me confused why two dependency definitions, too.
@fengzhou-msft Could you answer the question by Calvin?

@yonzhan yonzhan modified the milestones: S186, S187 May 1, 2021
@panchagnula
Copy link
Contributor

@calvinsID looks like this change has some merge conflicts.

@calvinsID calvinsID force-pushed the github-access-token branch from 94b3dd5 to fda252b Compare May 3, 2021 15:18
@calvinsID calvinsID changed the title [AppService] az appservice: Add function to retrieve users github personal access token, add pieces for Github Actions support [AppService] az appservice: Add function to retrieve users github personal access token and add pieces for Github Actions support May 3, 2021
@calvinsID calvinsID changed the title [AppService] az appservice: Add function to retrieve users github personal access token and add pieces for Github Actions support [AppService] Add function to retrieve users github personal access token and add pieces for Github Actions support May 3, 2021
@calvinsID calvinsID changed the title [AppService] Add function to retrieve users github personal access token and add pieces for Github Actions support [AppService] Add function to retrieve users github personal access token May 3, 2021
@calvinsID calvinsID changed the title [AppService] Add function to retrieve users github personal access token [AppService] Add ability to retrieve users github personal access token May 3, 2021
@calvinsID calvinsID changed the title [AppService] Add ability to retrieve users github personal access token [AppService] az appservice: Add function to retrieve users github personal access token May 3, 2021
@calvinsID
Copy link
Contributor Author

@qwordy wondering if u know how I can fix the failing check?

@qwordy
Copy link
Member

qwordy commented May 7, 2021

@qwordy wondering if u know how I can fix the failing check?

Just rerun

'jsmin~=2.2.2',
'jsondiff==1.2.0',
'packaging~=20.9',
'PyGithub==1.38',
Copy link
Member

@jiasli jiasli Jun 21, 2021

Choose a reason for hiding this comment

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

github module is not imported anywhere. Is this dependency necessary?

The pinned version causes problem when being installed with the latest PyGithub (#18548):

azure-cli 2.24.2 requires PyGithub==1.38, but you'll have pygithub 1.55 which is incompatible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I should have added it to this PR instead, it will be used in this one #18261. And yes we shouldn't pin, we can use ~= and unpin. Thanks for bringing this up

Copy link
Member

Choose a reason for hiding this comment

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

Could you change the dependency to use ~= instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

App Services az appservice

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants