Skip to content
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

Add Ruby2.5 runtime support #3725

Merged
merged 2 commits into from
Jul 30, 2018
Merged

Add Ruby2.5 runtime support #3725

merged 2 commits into from
Jul 30, 2018

Conversation

remore
Copy link
Contributor

@remore remore commented Jun 6, 2018

This PR adds a new ruby:2.5 kind to allow Ruby methods like this:

def main(param)
  { :result => param.upcase }
end

Description

This PR is quite similar to #2415, but this time I did it in Ruby. I do believe there are certain number of developers who'd love to pick Ruby as a primary language (see #2046 for an instance).

Currently Ruby runtime is tentatively hosted at remore/openwhisk-runtime-ruby, but needless to say I'd love to move/transfer this to anywhere The Apache Software Foundation manage.

Related issue and scope

#2046 is most likely related

My changes affect the following components

  • API
  • Controller
  • Message Bus (e.g., Kafka)
  • Loadbalancer
  • Invoker
  • Intrinsic actions (e.g., sequences, conductors)
  • Data stores (e.g., CouchDB)
  • Tests
  • Deployment
  • CLI
  • General tooling
  • Documentation

Types of changes

  • Bug fix (generally a non-breaking change which closes an issue).
  • Enhancement or new feature (adds new functionality).
  • Breaking change (a bug fix or enhancement which changes existing behavior).

Checklist:

  • I signed an Apache CLA.
  • I reviewed the style guides and followed the recommendations (Travis CI will check :).
  • I added tests to cover my changes.
  • My changes require further changes to the documentation.
  • I updated the documentation where necessary.

@rabbah
Copy link
Member

rabbah commented Jun 6, 2018

Thanks for this contribution!

Two comments:

  1. The container tests should be in the runtime repository and we should generally remove the duplication here; @csantanapr I think we were planning to do this already?

  2. Please send an email to the list about the runtime (perhaps including a small example on how to try it). This pr Adjust invoker playbook to pull docker images when a prefix and tag is specified. #3680 will actually make it possible to try using your image on dockerhub. @csantanapr are you ok with merging that now?

Following the email we can propose creating a repository to accept the runtime into a new repository.

@@ -0,0 +1 @@
FROM openwhisk/action-ruby-v2.5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With #3680 we won’t need this (and can remove all the others like it).

cc @csantanapr

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I will see for how #3680 goes then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't canceled this change yet since it looks like Dockerfile and build.gradle files in the actionRuntime/ folder still remains for other language runtimes while #3680 has been merged

@@ -25,7 +25,7 @@ This page outlines how to configure parameters when deploying packages and actio

### Passing parameters to an action at invoke time

Parameters can be passed to the action when it is invoked. These examples use JavaScript but all the other languages work the same way (see documentation on [Swift actions](./actions.md#creating-swift-actions), [Python actions](./actions.mdcreating-python-actions), [Java actions](./actions.mdcreating-java-actions), [PHP actions](./actions.mdcreating-php-actions), [Docker actions](./actions.mdcreating-docker-actions) or [Go actions](./actions.mdcreating-go-actions) as appropriate for more detailed examples).
Parameters can be passed to the action when it is invoked. These examples use JavaScript but all the other languages work the same way (see documentation on [Swift actions](./actions.md#creating-swift-actions), [Python actions](./actions.mdcreating-python-actions), [Java actions](./actions.mdcreating-java-actions), [PHP actions](./actions.mdcreating-php-actions), [Ruby actions](./actions.md#creating-ruby-actions), [Docker actions](./actions.mdcreating-docker-actions) or [Go actions](./actions.mdcreating-go-actions) as appropriate for more detailed examples).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This list will just keep getting longer. Perhaps we should provide just one reference to the index above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated accordingly.

@@ -434,6 +434,14 @@ The following Composer packages are also available:
- guzzlehttp/guzzle v6.3.0
- ramsey/uuid v3.6.1

## Ruby actions

Ruby actions are executed using Ruby 2.5. To use this runtime, specify the `wsk` CLI parameter `--kind ruby:2.5` when creating or updating an action. This is the default when creating an action with file that has a `.rb` extension.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You’ll have modified the cli then to recognize this extension. I have an alternate proposal here: apache/openwhisk-cli#286

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I will keep my eyes open on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rabbah I have pushed a PR to workaround tentatively.


A few Ruby gems such as `mechanize` and `jwt` are available in addition to the default and bundled gems. See Dockerfile of ruby runtime image for more information.

Last but not least, you can use arbitrary gem so long as you use zipped actions to package all the dependencies. doc/actions.md will explain how to do this.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

settings.gradle Outdated
@@ -28,6 +28,7 @@ include 'actionRuntimes:swift3.1.1Action'
include 'actionRuntimes:swift4.1Action'
include 'actionRuntimes:javaAction'
include 'actionRuntimes:php7.1Action'
include 'actionRuntimes:ruby2.5Action'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@csantanapr we can also drop all of these I think.

@rabbah rabbah added runtime review Review for this PR has been requested and yet needs to be done. labels Jun 6, 2018
@remore
Copy link
Contributor Author

remore commented Jun 6, 2018

@rabbah Thanks for catching this up. I have just sent an email to [email protected] but correct me if the email address was wrong.

@@ -25,7 +25,7 @@ This page outlines how to configure parameters when deploying packages and actio

### Passing parameters to an action at invoke time

Parameters can be passed to the action when it is invoked. These examples use JavaScript but all the other languages work the same way (see documentation on [Swift actions](./actions.md#creating-swift-actions), [Python actions](./actions.mdcreating-python-actions), [Java actions](./actions.mdcreating-java-actions), [PHP actions](./actions.mdcreating-php-actions), [Docker actions](./actions.mdcreating-docker-actions) or [Go actions](./actions.mdcreating-go-actions) as appropriate for more detailed examples).
Parameters can be passed to the action when it is invoked. These examples use JavaScript but all the other languages work the same way (see documentation on [this page](./actions.md) as appropriate for more detailed examples).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for more details examples ... that are language specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@@ -0,0 +1,408 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will also go with #3467.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I will remove this once #3467 get merged.

Copy link
Contributor Author

@remore remore Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed this with 3cc287c

@@ -0,0 +1,2 @@
ext.dockerImageName = 'action-ruby-v2.5'
Copy link
Member

@rabbah rabbah Jun 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this also becomes unnecessary - same comment as dockerfile.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it🙋‍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as this applies here

"kind": "ruby:2.5",
"default": true,
"deprecated": false,
"image": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with #3680, the image prefix and tag should be specified explicitly (to openwhisk and latest respectively).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made a change as a result of #3680

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed prefix and name temporarily to workaround the build failure. (thank you @rabbah for the quick advice to fix this)

Juice10 added a commit to Juice10/incubator-openwhisk that referenced this pull request Jun 14, 2018
Travis CI was kicking up the following error in apache#3725
```
Scan detected 1 error(s) in 1 file(s):
  [/home/travis/build/apache/incubator-openwhisk/tools/travis/../../actionRuntimes/ruby2.5Action/build.gradle]:
       1: file does not include required license header.
Scan detected 1 error(s) in 1 file(s):
```

Super easy fix, thought I'd help you out. Thanks for working on Ruby runtime support!
@codecov-io
Copy link

codecov-io commented Jun 15, 2018

Codecov Report

Merging #3725 into master will decrease coverage by 4.69%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #3725     +/-   ##
=========================================
- Coverage    75.9%   71.21%   -4.7%     
=========================================
  Files         147      147             
  Lines        7048     7048             
  Branches      422      422             
=========================================
- Hits         5350     5019    -331     
- Misses       1698     2029    +331
Impacted Files Coverage Δ
...core/database/cosmosdb/RxObservableImplicits.scala 0% <0%> (-100%) ⬇️
...core/database/cosmosdb/CosmosDBArtifactStore.scala 0% <0%> (-95.1%) ⬇️
...sk/core/database/cosmosdb/CosmosDBViewMapper.scala 0% <0%> (-92.6%) ⬇️
...whisk/core/database/cosmosdb/CosmosDBSupport.scala 0% <0%> (-81.82%) ⬇️
...abase/cosmosdb/CosmosDBArtifactStoreProvider.scala 0% <0%> (-58.83%) ⬇️
...la/whisk/core/database/cosmosdb/CosmosDBUtil.scala 92% <0%> (-4%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7000687...95a4414. Read the comment docs.

Copy link
Member

@rabbah rabbah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

both noted PRs are now committed eschewing several files in this PR.

@rabbah rabbah force-pushed the add-ruby-as-a-kind branch 2 times, most recently from b9699cf to 9032997 Compare June 26, 2018 03:08
@rabbah
Copy link
Member

rabbah commented Jun 26, 2018

@remore i squashed and rebased this to master to resolve a new conflicts with docs i updated.

@csantanapr we'll need to document the examples to be included here for system tests; should we just test a hello world here and defer the rest (including unicode) to the runtime tests?

@rabbah rabbah force-pushed the add-ruby-as-a-kind branch 2 times, most recently from d1de77f to c46c3ad Compare July 3, 2018 15:12
@rabbah
Copy link
Member

rabbah commented Jul 3, 2018

@remore @csantanapr I rebased this to pick up the latest refactoring.

docs/actions.md Outdated
@@ -54,13 +54,14 @@ the [Docker](docker-actions.md) action or [native binary](docker-actions.md#crea
paths more suitable. Multiple actions may be composed together to create a longer processing pipeline called a
[sequence](#creating-action-sequences). A more advanced form of composition is described [here](conductors.md).

* [JavaScript](actions-node.md)
* [Python](actions-python.md)
* [Go](actions-go.md)
Copy link
Member

@rabbah rabbah Jul 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akrabat @csantanapr I reordered these alphabetically (but keeping docker actions last). FYI.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alphabetical ordering makes sense to me.

Copy link
Member

@rabbah rabbah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually we will need a hello.rb files under tests/dat/actions.md for the automated system tests (that will be added).

it would be nice to automatically extract the example from the doc and turn it into a test :)... not for this pr

@rabbah rabbah mentioned this pull request Jul 3, 2018
21 tasks
@rabbah rabbah force-pushed the add-ruby-as-a-kind branch 3 times, most recently from f367e6f to 02fb9e5 Compare July 6, 2018 02:33
@jthomas
Copy link
Member

jthomas commented Jul 12, 2018

Looks like this PR is almost ready to go but the doc re-org needs the Ruby action details in a separate markdown file now? @remore are you able to do this small change and we can get it merged?
thanks for all the hard work on this.

@rabbah
Copy link
Member

rabbah commented Jul 12, 2018

@jthomas we need to do a couple of things apart form updating this pr:

  1. make sure the ruby runtime repo adopts and passes the basic runtimes test suite
  2. move the runtime remote to an apache repo (requires INFRA to create the repo)
  3. successfully building and tagging the ruby runtime from that repo
  4. changing the runtime manifest to use openwhisk/ruby runtime instead of @remore's personal docker prefix

@remore
Copy link
Contributor Author

remore commented Jul 13, 2018

are you able to do this small change and we can get it merged?

Yes for sure, I have just made a few changes to get everything organized in the ruby-actions.md.
Thank you for shedding the light on this. @jthomas

make sure the ruby runtime repo adopts and passes the basic runtimes test suite

Here is the log output of test code which is executed in my laptop with the latest code today just FYI @rabbah

vagrant@ubuntu-xenial:~/openwhisk-runtime-ruby$ ./gradlew :tests:test --tests *actionContainers.Ruby25ActionContainerTests*

> Task :tests:test

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should run a RUBY script STANDARD_OUT
    test router? false

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should run a RUBY script PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should run and report an error for script not returning a json object PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should run a RUBY action and handle unicode in source, input params, logs, and result PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should run a RUBY script and confirm expected environment variables PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should fail to initialize with bad code PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should fail to initialize with no code PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should return some error on action error PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should support application errors PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should fail gracefully when an action has a TypeError exception PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should support the documentation examples (1) PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should have mechanize and activesupport gems available PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should support large-ish actions PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should support zip-encoded packages PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should fail gracefully on invalid zip files PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should fail gracefully on valid zip files that are not actions PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should fail gracefully on valid zip files with invalid code in main.rb PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should support actions using non-default entry point PASSED

runtime.actionContainers.Ruby25ActionContainerTests > action-ruby-v2.5 should support zipped actions using non-default entry point PASSED


BUILD SUCCESSFUL in 48s
3 actionable tasks: 1 executed, 2 up-to-date

move the runtime remote to an apache repo (requires INFRA to create the repo)

I'm willing to proceed with these steps only if I'm eligible. If so, please let me know.

@rabbah
Copy link
Member

rabbah commented Jul 13, 2018

There some conflicts to resolve on the PR.

@rabbah
Copy link
Member

rabbah commented Jul 13, 2018

@remore I opened 3 issues against the ruby repo that we’ll need to also address.

@remore
Copy link
Contributor Author

remore commented Jul 13, 2018

@rabbah the conflict has been fixed. I will take a look at those issues.

@remore
Copy link
Contributor Author

remore commented Jul 13, 2018

FYI: I have just squashed as well to make both git tree view and log messages cleaner. (I have just realized that my commit messages were not following comment message convention here. My apologies.)

@rabbah
Copy link
Member

rabbah commented Jul 13, 2018

Repository requested: https://issues.apache.org/jira/browse/INFRA-16764

@jthomas
Copy link
Member

jthomas commented Jul 17, 2018

@remore
Copy link
Contributor Author

remore commented Jul 17, 2018

The first PR has been sent:
apache/openwhisk-runtime-ruby#1

@rabbah
Copy link
Member

rabbah commented Jul 29, 2018

@remore we (@csantanapr or i) will be fixing the travis publishing step in the ruby repo shortly.
can you add the expected sniff test https://github.com/apache/incubator-openwhisk/blob/master/docs/actions-new.md#the-test-action.

and go ahead and change the image prefix s/remore/openwhisk - travis will fail for now but i'll rebuild it when the image is published.

this gets the PR ready for merging.

"deprecated": false,
"image": {
"prefix": "remore",
"name": "openwhisk-runtime-ruby",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the image name should be action-ruby-v2.5 per the naming convention.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"default": true,
"deprecated": false,
"image": {
"prefix": "remore",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

go ahead and change this image prefix to openwhisk.

Copy link
Member

@rabbah rabbah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@remore I went ahead and made the changes.

@remore
Copy link
Contributor Author

remore commented Jul 30, 2018

@rabbah Thank you for going ahead about this.

this gets the PR ready for merging.

Can't wait to see that happen.

@rabbah
Copy link
Member

rabbah commented Jul 30, 2018

image now published to dockerhub https://hub.docker.com/r/openwhisk/action-ruby-v2.5 (thanks @csantanapr).

@rabbah rabbah self-assigned this Jul 30, 2018
@rabbah rabbah merged commit 35cf728 into apache:master Jul 30, 2018
@jthomas
Copy link
Member

jthomas commented Aug 6, 2018

Great to see this merged. 💯
@remore Would you like to do an announcement blog post for the OpenWhisk Medium channel?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Review for this PR has been requested and yet needs to be done. runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants