Ruby Build Initial Design #49
Conversation
|
|
||
| ## Interface/Implementation | ||
|
|
||
| Off hand, I envision the following commands as a starting point: |
There was a problem hiding this comment.
How do you envision the workflow actions to look like?
would it just be?
Action 1: CopySource
Action 2: bundle install # if no Gemfile.lock is present
Action 3: bundle install --deployment
That seems nice and simple to start out 👍
|
|
||
| ## Challenges | ||
|
|
||
| - Ensuring that builds happen in Ruby 2.5.x only. |
There was a problem hiding this comment.
Agreed, trying to start up a design here on that: #48
|
The overall design looks good to me 👍 . We just need to expand the implementation details to be workflow specific and not sam specific. |
Follows the NPM patterns, with the following open work: - Tests - Documentation - Resolve patterns to use for different directories that are needed and not needed. - Possible tweaking of Bundler options.
|
Linter breakage seems related to the fact that I haven't finalized how to approach the different directories in the standard actions - several won't apply, most likely. |
Matches CopySource action patterns.
|
Looking at the workflow. this looks good to me. With regards the TODO: windows issues,I think the integration tests (once added) should catch them, because we run the integ tests in both a linux and a windows env. |
|
Makes sense, I'm writing tests now. |
Other test types remain.
Basic tests to cover with and without dependencies. Additionally, refactored folder structure to match other runtimes, and wired up the workflow to make the integration tests pass.
Validates that we get an error from Bundler when dependencies conflict.
| 2. Building and deploying the user dependencies as a layer rather than as part of the code package. | ||
| - These also have slightly different folder pathing: | ||
| - Bundled dependencies are looked for in `/var/task/vendor/bundle/ruby/2.5.0` which is the default result of a `bundle install --deployment` followed by an upload. | ||
| - Layer dependencies are looked for in `/opt/ruby/gems/2.5.0`, so for a layer option would have to use a `--path` build or transform the folder structure slightly. |
There was a problem hiding this comment.
Yes this needs to happen, when we expand to building layers.
There was a problem hiding this comment.
Yup, this is just the "how" when we go there.
| import subprocess | ||
|
|
||
|
|
||
| class OSUtils(object): |
There was a problem hiding this comment.
Eventually we want to move this to a shared library, so all that workflows will have access to it. But not blocking on this right now.
There was a problem hiding this comment.
Totally agreed, I will fully admit I'm copying for convenience and 100% support merging to a shared library. Didn't want to do it myself since I'm tentative on my Python skills and don't want to step on your own structure changes.
|
|
||
| invoke_bundler = [self.bundler_exe] + args | ||
|
|
||
| LOG.debug("executing Bundler: %s", invoke_bundler) |
There was a problem hiding this comment.
Do you think adding absolute path helps here? just to show where bundler is coming from.
There was a problem hiding this comment.
Traditionally wouldn't be needed, but we can add it if that's important.
| """ | ||
| NAME = "RubyBundlerBuilder" | ||
|
|
||
| CAPABILITY = Capability(language="ruby", |
There was a problem hiding this comment.
we can add validation of ruby runtime in another PR.
There was a problem hiding this comment.
Yep, I commented in that other PR what Ruby would need. It's a really simple one-liner.
It's a bit lengthy, but seems to be required.
Need more escape characters, naturally.
Appveyor docs indicate this is the case.
Probably breaking the integ tests, worked on my local machine but not Windows.
It should be on the path, but getting issues...
There's got to be a better way to do this, but I don't have a Windows box to test on...
Continuing to debug Windows remotely...
Getting closer...
Checking path concerns given the last error message I got.
New error!
This seems off, but one error message has this as a surface-level interpretation.
Based on local testing.
This is the intended test, the root issue should now be solved.
| ```shell | ||
| # ensure you are using Ruby 2.5, for example with rbenv or rvm | ||
| bundle install # if no Gemfile.lock is present | ||
| bundle install --deployment |
There was a problem hiding this comment.
Should bundle install skip dev and test? --without development test?
There was a problem hiding this comment.
This is a fair shout. I think in an ideal world users could pass in their own options, but this would be a reasonable default.
| The basic scope of a `sam build` script for Ruby would be as a shortcut for this, while performing some housekeeping steps: | ||
|
|
||
| - Skipping the initial `bundle install` if a Gemfile.lock file is present. | ||
| - Ensuring that `ruby --version` matches `/^ ruby 2\.5\./` |
There was a problem hiding this comment.
Can we make sure we get to as close to 100% coverage as possible:
aws_lambda_builders/workflows/ruby_bundler/__init__.py 1 0 0 0 100%
aws_lambda_builders/workflows/ruby_bundler/actions.py 32 0 0 0 100%
aws_lambda_builders/workflows/ruby_bundler/bundler.py 26 1 10 1 94% 30, 29->30
aws_lambda_builders/workflows/ruby_bundler/utils.py 21 8 0 0 62% 19-20, 23-24, 27, 31, 34, 37
aws_lambda_builders/workflows/ruby_bundler/workflow.py 17 0 2 1 95% 42->45
.gitignore seems to have come after tests were added.
Adds `--without development test` to default option set.
- Bundler as dependency manager.
Initial design for adding Ruby support to
sam buildand related commands.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.