-
Notifications
You must be signed in to change notification settings - Fork 332
Python client: add license check #2580
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
Python client: add license check #2580
Conversation
flyrain
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 Thanks for working on it, @MonkeyCanCode !
HonahX
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for working on this.
I've verified this can catch non-supported license in deps. e.g.
--- Starting license compliance check ---
license GNU Lesser General Public License v2 or later (LGPLv2+) not in allow-only licenses was found for package chardet:5.2.0
make: *** [client-license-check] Error 1
| .PHONY: client-license-check | ||
| client-license-check: client-setup-env ## Run license compliance check | ||
| @echo "--- Starting license compliance check ---" | ||
| @$(ACTIVATE_AND_CD) && pip-licenses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow this does not work for me in an old poetry environment.
pip-licenses: command not found
But I've verified that a clean install will work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the previous poetry env doesn't have this dependency installed as I added it last night via this PR. In case if u want that env to work, u can source the venv then run poetry command to install all again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I ran make install-dependencies and I saw poetry explicitly said it installed pip-licenses yet it still could not found it. Could be some other weird issue in that env. So I ends up using a clean environment to verify : )
Using poetry run pip-licenses will work btw
|
@MonkeyCanCode the merge broke CI on |
NVM, merged your fix. |
This is requested by @snazy a while back via #822 and @DaniilRoman did the initial implementation via #1102. This is the PR for merged the changes from sample PR with our GH action and Makefile.
There are a lot more allowed licenses from ASF (https://www.apache.org/legal/resolved.html#category-x) and a bunch for which should't be included as well. For now, I put the allow list with the packages that are currently being used.