-
Notifications
You must be signed in to change notification settings - Fork 187
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
Py deps upgrade, TF upgrade, TF fixes #174
base: master
Are you sure you want to change the base?
Conversation
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.
a few comments but lgtm
I checked the logs of the CI job but I can't see any meaningful error message. I see that both jobs on Py 3.6 and Py 3.7 have errored, but I can't see the cause. Maybe you have some hints on how to dig deeper @ryandeivert ? Thanks! |
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.
Awesome, thanks for the contribution!
but I can't see the cause
You can't see this page? https://travis-ci.org/github/airbnb/binaryalert/jobs/756818009
You should be able to click "details" on the X shown on the commit, but maybe it's not public for some reason
$ python --version
Python 3.6.7
$ pip --version
pip 20.1.1 from /home/travis/virtualenv/python3.6.7/lib/python3.6/site-packages/pip (python 3.6)
install
37.21s$ pip3 install -r requirements.txt
0.28s$ tests/ci_tests.sh
~~~~~~~~~~ [1] Bandit Security Linting ~~~~~~~~~~
[main] INFO Found project level .bandit file: ./.bandit
[main] INFO Using command line arg for excluded paths
[main] INFO Using ini file for skipped tests
[main] INFO Using command line arg for selected targets
[main] INFO Using command line arg for recursive scan
[main] INFO Using command line arg for aggregate output type
[main] INFO Using command line arg for max code lines output for issue
[main] INFO Using command line arg for severity level
[main] INFO Using command line arg for confidence level
[main] INFO Using command line arg for output format
[main] INFO Using command line arg for output file
[main] INFO profile include tests: None
[main] INFO profile exclude tests: None
[main] INFO cli include tests: None
[main] INFO cli exclude tests: B303,B322,B404,B603,B607
[main] ERROR Unknown test found in profile: B322
The command "tests/ci_tests.sh" exited with 2.
Looks like the CI environment will need to be updated with the newer version of Python, maybe? You can configure CI with the .travis.yml
file
The real error is probably just the exclusion shown here:
[main] INFO cli exclude tests: B303,B322,B404,B603,B607
[main] ERROR Unknown test found in profile: B322
@@ -1,4 +1,4 @@ | |||
#!/usr/bin/env python3 | |||
#!/usr/bin/env python |
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.
If I remember right, this is an auto-generated file; did it change with the latest version of sphinx
?
@@ -47,7 +47,7 @@ resource "aws_lambda_function" "function" { | |||
description = var.description | |||
handler = var.handler | |||
role = aws_iam_role.role[0].arn | |||
runtime = "python3.6" | |||
runtime = "python3.7" |
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.
AWS Lambda has Python3.8 available, should we use that?
to: @airbnb/binaryalert-maintainers
resolves #145
resolves #173
resolves #167
resolves #166
resolves #154
resolves #153
resolves #152
resolves #142
resolves #144
Background
I see that there are many pending PR and open issues, so I tried to take the most important into account into one, comprehensive PR.
Changes
aws_account_name
to solve Deploy errors using the IAM Group #145/usr/bin/env python3
into/usr/bin/env python
to be venv friendly (Py3 should be the default)Testing
python manage.py unit_test