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 PHP 7.1 as a kind #2415

Merged
merged 13 commits into from Jul 24, 2017

Conversation

Projects
None yet
5 participants
@akrabat
Copy link
Member

commented Jun 22, 2017

This adds a new php:7.1 kind to allow PHP actions like this:

<?php
function main(array $args) : array
{
    $name = $args["name"] ?? "stranger";
    $greeting = "Hello $name!";
    return ["greeting" => $greeting];
}

The PHP runtime is based on the official php:7.1-alpine one with the following extensions added:

  • opcache
  • mysqli
  • pdo_mysql
  • pdo_pgsql
  • intl
  • bcmath
  • zip
  • gd
  • soap

Two Composer packages are also included:

  • guzzlehttp/guzzle - A very popular HTTP client
  • ramsey/uuid - The de facto UUID generator

Suggestions welcome on what should be included in these lists (and of course on the code itself!)

How to test

We need a working local installation. The easiest way to do this is with Vagrant, so that's what this set of steps uses.

  1. Check out the source code

     $ git clone https://github.com/apache/incubator-openwhisk.git
     $ cd incubator-openwhisk
    
  2. Now change to my branch

     $ git remote add akrabat https://github.com/akrabat/openwhisk.git
     $ git fetch akrabat
     $ git checkout php-kind
    
  3. Run up the Vagrant box

     $ cd tools/vagrant
     $ vagrant up
    

    This takes a while!

    I'm not sure what Vagrant is required, but I'm using Vagrant 1.9.1 with VirtualBox 5.1.18. I'm not aware of any other requirements.

    You should see output at the end like:

     ==> default: ++ wsk action invoke /whisk.system/utils/echo -p message hello --result
     ==> default: {
     ==> default:     "message": "hello"
     ==> default: }
     ==> default: +++ date
     ==> default: ++ echo 'Sun Jul 23 14:28:31 UTC 2017: build-deploy-end'
    
  4. Build the PHP Docker container

     $ vagrant ssh
     $ cd openwhisk
     $ ./gradlew :core:php7.1Action:distDocker -PdockerImagePrefix=openwhisk
    

    This takes a little while while it compiles various PHP bits and bobs.

  5. Testing a PHP action

    The vagrant box has a compatible wsk in it, so we can use it:

     $ cd ~
     $ cat << 'EOF' >> hello.php
     <?php
     function main(array $args) : array
     {
         echo "Some output to the invocation log";
         $name = $args["name"] ?? "stranger";
         $greeting = "Hello $name!";
         return ["greeting" => $greeting];
     }
     EOF
     $ wsk -i action update hello hello.php --kind php:7.1
     ok: updated action hello
     $ wsk -i action invoke hello -r -p name World
    

    If you see:

     {
         "greeting": "Hello World!"
     }
    

    Then it worked.

@markusthoemmes

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2017

Without an in depth review: Looks awesomely executed! Thanks for this!

@akrabat akrabat force-pushed the akrabat:php-kind branch from 4012db3 to f00e1a8 Jun 22, 2017

@akrabat

This comment has been minimized.

Copy link
Member Author

commented Jun 23, 2017

Travis is reporting [WARNING]: Unable to find '/home/travis/build/apache/incubator-openwhisk/ansible/db_local.ini' in expected paths..

Not sure how to fix?

@csantanapr

This comment has been minimized.

Copy link
Contributor

commented Jun 23, 2017

@akrabat This is an awesome contribution !!!
I will give it a try when I have chance.

@csantanapr

This comment has been minimized.

Copy link
Contributor

commented Jun 23, 2017

@akrabat the problem is nothing to do with ansible/db_local.ini
The problem is that the /tools/travis/build.sh failed on scancode

  [./tests/src/test/scala/actionContainers/Php71ActionContainerTests.scala]:
       1: file does not include required license header.
@@ -0,0 +1,470 @@
/**

This comment has been minimized.

Copy link
@csantanapr

csantanapr Jun 23, 2017

Contributor

I think it needs to be single /* to pass the scan

* See the License for the specific language governing permissions and
* limitations under the License.
*/
package actionContainers

This comment has been minimized.

Copy link
@csantanapr

csantanapr Jun 23, 2017

Contributor

add space after the license, just in case

@akrabat

This comment has been minimized.

Copy link
Member Author

commented Jun 23, 2017

That'll teach me to read the text in bright red/pink!

@akrabat akrabat force-pushed the akrabat:php-kind branch 3 times, most recently from cd57afa to ecd706a Jun 24, 2017

@csantanapr
Copy link
Contributor

left a comment

adjust for dynamic invoker images

@@ -72,6 +72,11 @@ runtimesManifest:
image:
name: "java8action"
default: true
php:

This comment has been minimized.

Copy link
@csantanapr

csantanapr Jun 27, 2017

Contributor

add deprecated: false

@@ -13,6 +13,7 @@
- '{{ docker_image_prefix }}/python3action'
- '{{ docker_image_prefix }}/swift3action'
- '{{ docker_image_prefix }}/java8action'
- '{{ docker_image_prefix }}/action-php-v7.1'

This comment has been minimized.

Copy link
@csantanapr

csantanapr Jun 27, 2017

Contributor

No longer need it, now the list is not hard coded

@akrabat akrabat force-pushed the akrabat:php-kind branch from a4ab772 to 53ab7e7 Jun 29, 2017

@akrabat

This comment has been minimized.

Copy link
Member Author

commented Jun 29, 2017

@csantanapr Rebased to master & addressed comments.

@csantanapr

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2017

@akrabat this is a significant contribution adding a new language support.
What do you think if you could socialize/post it to the dev list?

People can try and maybe comment on this PR with any feedback

@csantanapr

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2017

Also I think you need to rebase/merge to upstream/master

@akrabat akrabat force-pushed the akrabat:php-kind branch from eee751a to 76ecb72 Jul 2, 2017

@akrabat

This comment has been minimized.

Copy link
Member Author

commented Jul 2, 2017

Rebased.

@darrenmothersele

This comment has been minimized.

Copy link

commented Jul 4, 2017

This is an exciting development.

Do you think it could be possible to support dynamic composer dependencies?
Perhaps something like http://melody.sensiolabs.org/

@akrabat

This comment has been minimized.

Copy link
Member Author

commented Jul 4, 2017

@akrabat

This comment has been minimized.

Copy link
Member Author

commented Jul 12, 2017

Ping @lornajane for code review & thoughts on list of PHP extensions and Composer modules to include.

@darrenmothersele

This comment has been minimized.

Copy link

commented Jul 12, 2017

+1 for zip file with vendor folder, if pulling dependencies from composer file is not viable.

-1 for including a default set of composer packages.

@akrabat

This comment has been minimized.

Copy link
Member Author

commented Jul 12, 2017

Every other OpenWhisk action container ships with some default components - the most obvious ones being easy abilities to invoke other actions. e.g. Swift ships with Kitura-nat, SwiftyJSON and swift-watson-sdk.

I think the expectation is that the PHP action should follow suit, so included Guzzle and Uuid. I'm open to arguments on this though.

We are completely in agreement that if the user ships their own vendor folder, then it should "win" though :)

@darrenmothersele

This comment has been minimized.

Copy link

commented Jul 12, 2017

Re: extensions, this set would be good:

  • mysqli
  • xml
  • gd
  • openssl
  • json
  • curl
  • mbstring
  • PDO
  • pdo_mysql
  • pdo_sqlite
  • tokenizer
  • ctype
  • bcmath
  • intl
  • mcrypt

Sources:
https://www.drupal.org/docs/7/system-requirements/php#extensions
http://symfony.com/doc/current/reference/requirements.html
https://laravel.com/docs/5.4/installation#server-requirements

@rabbah

This comment has been minimized.

Copy link
Member

commented Jul 12, 2017

The built in packages are convenient - less zip files for the initial ramp up. But it creates a maintenance issue: when do you pick up updates to the packages (minor/patch level only?) and not break existing actions using the "kind". That is: is the kind itself semantically verionsed?

@csantanapr, @bjustin-ibm FYI.

@akrabat

This comment has been minimized.

Copy link
Member Author

commented Jul 13, 2017

@darrenmothersele Everything on your list except mcrypt is included.

I have not included mcrypt it is deprecated in PHP 7.1 as it has been unmaintained since 2007 and abandoned since 2011.

It is completely removed from PHP 7.2 and I don't see any point in including it.

@darrenmothersele

This comment has been minimized.

Copy link

commented Jul 13, 2017

@akrabat cool that sounds good!

akrabat added some commits Jun 17, 2017

Implement PHP 7.1 kind
Build the Docker container from php:7.1-alpine and implement the HTTP
server using PHP's built in server.

Note that when using a zip file, the router requires that the `main`
function is stored in `index.php`.
Output runner text if exit code is 1
The runner sets the exit code to 1 if it has set the last line of stdout
to a string suitable for presentation to the user. Therefore, if the
exit code is not one, then display a generic message.
Update PHP action environment
* Add PHP_VERSION for consistency with other kinds.
* Add WHISK_INPUT for consistency with Swift kind.
Consolidate output of router to top
Make it clearer what the router outputs by ensuring all the code that
does this at the top.

Also, clarify that the body sent back to the client is always JSON and
add per-function documentation.
Tidy up log out to remove blank lines
Also remove the duplicated last line if we've sent it to the client.
Output stderr & stdout when checking for main()
If there's a runtime error in the action (i.e. not spotted by linter),
then looking for the main() function will find it. Render the error to
the logs so that the user knows what's happened.
Ensure that a PHP zip file can contain vendor
If the PHP vendor file has a vendor directory, then this directory needs
to be used rather than the one supplied in the action container.

To do this, we require src/vendor/autoload.php which will exist if the
zip file contains it. For the cases where zip file does not contain a
vendor folder and when running a non-binary code action, we move the
container's vendor folder into src/.

@akrabat akrabat force-pushed the akrabat:php-kind branch from 1d835c6 to d1baf9d Jul 16, 2017

@akrabat

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2017

Rebased to pick up #2437

@rabbah rabbah merged commit 9401f8b into apache:master Jul 24, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

keunseob added a commit to keunseob/incubator-openwhisk that referenced this pull request Jul 28, 2017

Add PHP 7.1 as a kind (apache#2415)
* Implement PHP 7.1 kind
* Add tests for PHP 7.1 action
* Add PHP action documentation

Build the Docker container from php:7.1-alpine and implement the HTTP
server using PHP's built in server.

Note that when using a zip file, the router requires that the `main`
function is stored in `index.php`.

Note about the runner:
The runner sets the exit code to 1 if it has set the last line of stdout
to a string suitable for presentation to the user. Therefore, if the
exit code is not one, then display a generic message.

If there's a runtime error in the action (i.e. not spotted by linter),
then looking for the main() function will find it. Render the error to
the logs so that the user knows what's happened.

Note about vendor folder in a PHP zip:
If the PHP vendor file has a vendor directory, then this directory needs
to be used rather than the one supplied in the action container.

To do this, we require src/vendor/autoload.php which will exist if the
zip file contains it. For the two cases where (1) zip file does not contain a
vendor folder, or (2) when running a non-binary code action, we move the
container's vendor folder into src/.

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Aug 1, 2017

Add PHP 7.1 as a kind (apache#2415)
* Implement PHP 7.1 kind
* Add tests for PHP 7.1 action
* Add PHP action documentation

Build the Docker container from php:7.1-alpine and implement the HTTP
server using PHP's built in server.

Note that when using a zip file, the router requires that the `main`
function is stored in `index.php`.

Note about the runner:
The runner sets the exit code to 1 if it has set the last line of stdout
to a string suitable for presentation to the user. Therefore, if the
exit code is not one, then display a generic message.

If there's a runtime error in the action (i.e. not spotted by linter),
then looking for the main() function will find it. Render the error to
the logs so that the user knows what's happened.

Note about vendor folder in a PHP zip:
If the PHP vendor file has a vendor directory, then this directory needs
to be used rather than the one supplied in the action container.

To do this, we require src/vendor/autoload.php which will exist if the
zip file contains it. For the two cases where (1) zip file does not contain a
vendor folder, or (2) when running a non-binary code action, we move the
container's vendor folder into src/.

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Sep 18, 2017

Add PHP 7.1 as a kind (apache#2415)
* Implement PHP 7.1 kind
* Add tests for PHP 7.1 action
* Add PHP action documentation

Build the Docker container from php:7.1-alpine and implement the HTTP
server using PHP's built in server.

Note that when using a zip file, the router requires that the `main`
function is stored in `index.php`.

Note about the runner:
The runner sets the exit code to 1 if it has set the last line of stdout
to a string suitable for presentation to the user. Therefore, if the
exit code is not one, then display a generic message.

If there's a runtime error in the action (i.e. not spotted by linter),
then looking for the main() function will find it. Render the error to
the logs so that the user knows what's happened.

Note about vendor folder in a PHP zip:
If the PHP vendor file has a vendor directory, then this directory needs
to be used rather than the one supplied in the action container.

To do this, we require src/vendor/autoload.php which will exist if the
zip file contains it. For the two cases where (1) zip file does not contain a
vendor folder, or (2) when running a non-binary code action, we move the
container's vendor folder into src/.

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 13, 2017

Add PHP 7.1 as a kind (apache#2415)
* Implement PHP 7.1 kind
* Add tests for PHP 7.1 action
* Add PHP action documentation

Build the Docker container from php:7.1-alpine and implement the HTTP
server using PHP's built in server.

Note that when using a zip file, the router requires that the `main`
function is stored in `index.php`.

Note about the runner:
The runner sets the exit code to 1 if it has set the last line of stdout
to a string suitable for presentation to the user. Therefore, if the
exit code is not one, then display a generic message.

If there's a runtime error in the action (i.e. not spotted by linter),
then looking for the main() function will find it. Render the error to
the logs so that the user knows what's happened.

Note about vendor folder in a PHP zip:
If the PHP vendor file has a vendor directory, then this directory needs
to be used rather than the one supplied in the action container.

To do this, we require src/vendor/autoload.php which will exist if the
zip file contains it. For the two cases where (1) zip file does not contain a
vendor folder, or (2) when running a non-binary code action, we move the
container's vendor folder into src/.

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 13, 2017

Add PHP 7.1 as a kind (apache#2415)
* Implement PHP 7.1 kind
* Add tests for PHP 7.1 action
* Add PHP action documentation

Build the Docker container from php:7.1-alpine and implement the HTTP
server using PHP's built in server.

Note that when using a zip file, the router requires that the `main`
function is stored in `index.php`.

Note about the runner:
The runner sets the exit code to 1 if it has set the last line of stdout
to a string suitable for presentation to the user. Therefore, if the
exit code is not one, then display a generic message.

If there's a runtime error in the action (i.e. not spotted by linter),
then looking for the main() function will find it. Render the error to
the logs so that the user knows what's happened.

Note about vendor folder in a PHP zip:
If the PHP vendor file has a vendor directory, then this directory needs
to be used rather than the one supplied in the action container.

To do this, we require src/vendor/autoload.php which will exist if the
zip file contains it. For the two cases where (1) zip file does not contain a
vendor folder, or (2) when running a non-binary code action, we move the
container's vendor folder into src/.

houshengbo pushed a commit to houshengbo/openwhisk that referenced this pull request Nov 14, 2017

Add PHP 7.1 as a kind (apache#2415)
* Implement PHP 7.1 kind
* Add tests for PHP 7.1 action
* Add PHP action documentation

Build the Docker container from php:7.1-alpine and implement the HTTP
server using PHP's built in server.

Note that when using a zip file, the router requires that the `main`
function is stored in `index.php`.

Note about the runner:
The runner sets the exit code to 1 if it has set the last line of stdout
to a string suitable for presentation to the user. Therefore, if the
exit code is not one, then display a generic message.

If there's a runtime error in the action (i.e. not spotted by linter),
then looking for the main() function will find it. Render the error to
the logs so that the user knows what's happened.

Note about vendor folder in a PHP zip:
If the PHP vendor file has a vendor directory, then this directory needs
to be used rather than the one supplied in the action container.

To do this, we require src/vendor/autoload.php which will exist if the
zip file contains it. For the two cases where (1) zip file does not contain a
vendor folder, or (2) when running a non-binary code action, we move the
container's vendor folder into src/.

@remore remore referenced this pull request Jun 6, 2018

Merged

Add Ruby2.5 runtime support #3725

9 of 20 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.