feat: Allow Ruby runtime to build without requiring a manifest#245
feat: Allow Ruby runtime to build without requiring a manifest#245mildaniel merged 20 commits intoaws:developfrom
Conversation
…flow more readable.
| if p.returncode != 0: | ||
| # Bundler has relevant information in stdout, not stderr. | ||
| raise BundlerExecutionError(message=out.decode("utf8").strip()) | ||
| if p.returncode == 10: |
There was a problem hiding this comment.
Although not explicitly documented by Bundler, the Bundler error codes can be seen here.
There was a problem hiding this comment.
I see; please add a comment for reference to others as this is non-standard.
Tangentially, could this be handled in a more standard way (similar to #243)? It seems Bundler always expects a Gemfile and isn't meaningful without it.
There was a problem hiding this comment.
We could maybe put the 10 in a constant like GEMFILE_NOT_FOUND (this would also remove the need for the comment at line 55).
You could put your link to error codes on the line before the constant.
There was a problem hiding this comment.
A discussion was had with the team regarding this change. This solution is non-standard because of the way Bundler behaves. Doing a similar approach to Python pip is dangerous because of Bundler's ability to search for a Gemfile in parent directories, meaning that it could break for users with non-standard project structures. Although unconventional, this approach was deemed safer.
I will go ahead and make the changes that Mathieu suggested to improve the clarity.
There was a problem hiding this comment.
Makes sense, thanks for the explanation. 👍
| Bundler error codes can be found here: | ||
| https://github.com/rubygems/bundler/blob/3f0638c6c8d340c2f2405ecb84eb3b39c433e36e/lib/bundler/errors.rb#L36 | ||
| """ | ||
| GEMFILE_NOT_FOUND = 10 |
There was a problem hiding this comment.
You need to put this outside of the function :)
* feat: Allow Python runtime to build without requiring a manifest (#243) * Allow Python to continue build without requirements.txt * Allow missing requirements.txt file for Python builds * Ruby optional Gemfile and test * Style changes * Add unit tests and additional comments * Fix assertLogs() not compatible with Python2.7 * Fix assertion string * Remove unused exception * Integ. test make sure no artifacts dir has no additional files after build * Kick off build * Check for manifest and skip package actions if not found * Revert Ruby changes until further discussion is had. Make Python workflow more readable. * Remove unused import * Whitespace fix * feat: Allow Ruby runtime to build without requiring a manifest (#245) * Allow Python to continue build without requirements.txt * Allow missing requirements.txt file for Python builds * Ruby optional Gemfile and test * Style changes * Add unit tests and additional comments * Fix assertLogs() not compatible with Python2.7 * Fix assertion string * Remove unused exception * Integ. test make sure no artifacts dir has no additional files after build * Kick off build * Check for manifest and skip package actions if not found * Revert Ruby changes until further discussion is had. Make Python workflow more readable. * Remove unused import * Whitespace fix * Readability changes for Ruby workflow * Remove magic number. Add link to Bundler error codes * Moved var declaration * docs: Guidance on integrating with Lambda Builders (#242) * fix: README - showcase Makefile support (#247) * Update VS2017 to VS2019 (#244) * fix: Pip not resolving local packages (#250) * Fix local packages not being built * Add int. test to catch future local dependency issues * Specify test requirements path from cwd * Removed redundant/superset pattern * Document the pip regex pattern change * Updated integ to match use case * Tests to check backward comp. * chore: bump version to 1.5.0 (#254) Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com> Co-authored-by: Giorgio Azzinnaro <giorgio.azzinnaro@gmail.com> Co-authored-by: Sriram Madapusi Vasudevan <3770774+sriram-mv@users.noreply.github.com>
4 participants
Which issue(s) does this change fix?
aws/aws-sam-cli#784
Why is this change necessary?
Allow Ruby runtime to build without requiring a manifest file.
How does it address the issue?
Logs a warning and continues to build Ruby projects that are missing a Gemfile.
What side effects does this change have?
Checklist:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.