Skip to content

Modify the unix script#3059

Merged
markusthoemmes merged 7 commits intoapache:masterfrom
chetanmeh:extend-classpath-3057
Dec 11, 2017
Merged

Modify the unix script#3059
markusthoemmes merged 7 commits intoapache:masterfrom
chetanmeh:extend-classpath-3057

Conversation

@chetanmeh
Copy link
Copy Markdown
Member

@chetanmeh chetanmeh commented Dec 5, 2017

Modifies the line containing CLASSPATH= in generated unix script file and add entries for

  1. $APP_HOME/ext-lib/* - To be used to add extra jars to classpath
  2. $APP_HOME/config - To be used to add custom application.conf

This PR is for issue #3057

Modifies the line containing `CLASSPATH=` in generated unix script file and add entries for

1. $APP_HOME/ext-lib/* - To be used to add extra jars to classpath
2. $APP_HOME/config - To be used to add custom application.conf
Comment thread build.gradle
}
}
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I googled for a bit and stumbled upon:

startScripts {
  classpath += files('ext-libs/*', 'config')
}

There seems to be a possibility to do this other than manipulating raw lines (which feels a bit rough). I'd need to look further into this, since gradle seems to apply some sanitizing unfortunately.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks @markusthoemmes for looking into this. I also saw that but it does not work as expected. Have a look at links provided in #3057 for background and hence used this approach

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@chetanmeh oh sorry, should've checked. Yeah your observations are exactly the same as mine, so we're clear on that 👍

Comment thread build.gradle Outdated

// Get original line and append it
// with the configuration directory.
original += ":\$APP_HOME/ext-lib/*:\$APP_HOME/config"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could use single quotes to circumenvent character escaping?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point. This was leftover of previous attempt which was also using a variable for path separator. With that not used we can use single quote. Updated that.

Copy link
Copy Markdown
Contributor

@markusthoemmes markusthoemmes left a comment

Choose a reason for hiding this comment

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

Some spelling mistakes left. Thanks for adding documentation on this 👍

Comment thread docs/spi.md Outdated

## Including the implementation

Base openwhisk docker images provide 2 extension points in classpath for including the implementation
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • in the classpath
  • missing . at the end.

Comment thread docs/spi.md Outdated

### Application Jars

The application jars can be added to `$APP_HOME/ext-lib` for e.g. in `openwhisk/controller` image the implementation jars can be added to `/controller/ext-lib` folder and for `openwhisk/invoker` they can be added to `/invoker/ext-lib` folder
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • to /controller/ext-lib folder
  • to /invoker/ext-lib folder
  • Missing . at the end.

Comment thread docs/spi.md Outdated

### Application Configuration

The configuration files can be added to `$APP_HOME/config` folder
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

  • folder
  • Missing . at the end.

@chetanmeh
Copy link
Copy Markdown
Member Author

Updated docs as per feedback

Copy link
Copy Markdown
Contributor

@markusthoemmes markusthoemmes left a comment

Choose a reason for hiding this comment

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

LGTM

@markusthoemmes markusthoemmes merged commit b57e40f into apache:master Dec 11, 2017
@chetanmeh chetanmeh deleted the extend-classpath-3057 branch March 21, 2018 13:16
BillZong pushed a commit to BillZong/openwhisk that referenced this pull request Nov 18, 2019
Modifies the line containing `CLASSPATH=` in generated unix script file and add entries for

1. $APP_HOME/ext-lib/* - To be used to add extra jars to classpath
2. $APP_HOME/config - To be used to add custom application.conf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants