-
Notifications
You must be signed in to change notification settings - Fork 148
fix: helpful error messages when runtime mismatch for build #37
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
Conversation
Is there an integ test that verifies this? If so, can you check the exception message there to double check and make sure the response from |
aws_lambda_builders/exceptions.py
Outdated
MESSAGE = "{language} executable found in your path does not " \ | ||
"match runtime. " \ | ||
"\n Expected version: {required_runtime}, Found version: {found_runtime}. " \ | ||
"\n Consider moving {required_runtime} up path hierarchy." \ |
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.
The "Consider..." line assumes customers do have Python somewhere else in their path. It is also not obvious how to do this for virtualenv/pyenv users. I would remove this line.
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.
done.
@@ -17,6 +17,7 @@ def validate_python_cmd(required_language, required_runtime_version): | |||
"python", | |||
"-c", | |||
"import sys; " | |||
"sys.stdout.write('python' + str(sys.version_info.major) + '.' + str(sys.version_info.minor)); " |
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.
haha neat ;)
if p.returncode != 0: | ||
raise MisMatchRuntimeError(language=required_language, | ||
required_runtime=required_runtime) | ||
required_runtime=required_runtime, | ||
found_runtime=str(found_runtime.decode('utf-8'))) |
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.
Is the decode valid in python2.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.
checked it with an integ test, and ran it under python2.
under python2, even if its already string and we decode. we get to unicode.
Python 2.7.10 (default, Feb 7 2017, 00:08:15)
Type "copyright", "credits" or "license" for more information.
IPython 5.8.0 -- An enhanced Interactive Python.
? -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help -> Python's own help system.
object? -> Details about 'object', use 'object??' for extra details.
In [1]: mystring="python"
In [2]: type(mystring)
Out[2]: str
In [3]: mystring.decode('utf-8')
Out[3]: u'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.
perfect
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.
Love the integ test :)
3f64de7
to
60e89ce
Compare
)" This reverts commit 608813c.
Fixing this involves following steps:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.