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

Add a check for linux version consistency #5755

Merged
merged 24 commits into from
Mar 8, 2018
Merged

Add a check for linux version consistency #5755

merged 24 commits into from
Mar 8, 2018

Conversation

yorek
Copy link
Contributor

@yorek yorek commented Mar 7, 2018

Followin up #5747

@yorek yorek requested a review from derekbekoe March 7, 2018 23:34
@promptws
Copy link

promptws commented Mar 7, 2018

View a preview at https://prompt.ws/r/Azure/azure-cli/5755
This is an experimental preview for @microsoft users.

@derekbekoe derekbekoe added Extensions `az extension` commands or extension infrastructure Packaging/Debian labels Mar 7, 2018
@derekbekoe derekbekoe added this to the Sprint 33 milestone Mar 7, 2018
@yorek
Copy link
Contributor Author

yorek commented Mar 8, 2018

saw that the build was failing on whitespace rules, fixed it, and already pushed the changes

@derekbekoe
Copy link
Member

Everything is good except for this:

************* Module azure.cli.command_modules.extension.custom
C:285, 0: Line too long (132/120) (line-too-long)
C:288, 0: Unnecessary parens after 'if' keyword (superfluous-parens)
C:290, 0: Line too long (159/120) (line-too-long)
C:291, 0: Line too long (150/120) (line-too-long)
W:282,15: Catching too general exception Exception (broad-except)
W:279,42: Using deprecated method linux_distribution() (deprecated-method)

@derekbekoe
Copy link
Member

For W:282,15: Catching too general exception Exception (broad-except), you can add # pylint: disable=line-too-long

Also, you can run pylint locally with azdev style --ci --module extension instead of having to wait for CI.

@yorek
Copy link
Contributor Author

yorek commented Mar 8, 2018

changed code to comply with pyling rules. Had to get rid of deprecated function.

@derekbekoe
Copy link
Member

In the last call to logger.warning, can you add current_linux_dist_name.strip()?

I just tried it and I see this.

az extension add -n webapp 
Linux distro check: Mismatch distribution name in /etc/apt/sources.list.d/azure-cli.list file
Linux distro check: If command fails, install the appropriate package for your distribution or change the above file accordingly.
Linux distro check: /etc/apt/sources.list.d/azure-cli.list has 'wheezy', current distro is 'xenial
'
An error occurred. Pip failed with status code 1. Use --debug for more information.

As you can see, the last ' ends up on a new line.

@derekbekoe
Copy link
Member

Same with Linux distro check: Reported by API: ... message.
So might be best to do return rel.strip() in get_lsb_release.

@derekbekoe
Copy link
Member

I verified that return rel.strip() would resolve the issue.

@derekbekoe derekbekoe merged commit 8c0de8e into Azure:dev Mar 8, 2018
@derekbekoe
Copy link
Member

Thanks for the contribution @yorek

@yorek
Copy link
Contributor Author

yorek commented Mar 8, 2018

You're welcome :)

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 Packaging/Debian
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants