Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
BIGTOP-2516: Add Zeppelin Charm #137
Conversation
c0s
reviewed
Aug 10, 2016
| @@ -0,0 +1,177 @@ | ||
| + |
c0s
Aug 10, 2016
Contributor
You don't need to include LICENSE file to each charm - it is already done at the top of the project.
kwmonroe
and others
added some commits
Jun 7, 2016
kwmonroe
suggested changes
Oct 4, 2016
Overall, I really dig this. The hadoop/hive/spark bits are super slick with the _add_overrides method. And i think the notebook registration stuff is destined for greatness as more charms provide zepp notebooks.
But, some parts suck. Fix those and ye shall have my approval.
| + | ||
| +Now relate Zeppelin to Java and the Hadoop cluster: | ||
| + | ||
| + juju add-relation zeppelin openjdk |
kwmonroe
Oct 4, 2016
Member
Openjdk is optional now and won't be present in the Bigtop bundles... which will make this add-relation step fail. Yank it (and the Java comment above).
| +of Juju, the syntax is `juju action fetch <action-id>`._ | ||
| + | ||
| + | ||
| +## Limitations |
kwmonroe
Oct 4, 2016
Member
Let's remove this section until juju-solutions#40 is fixed. Doesn't make any sense here since you can't change spark config on the bigtop-spark charm anyway.
| + # Dirs are handled by the bigtop deb. No need to call out to | ||
| + # dist_config to do that work. | ||
| + self.dist_config.add_users() | ||
| + self.trigger_bigtop() |
kwmonroe
Oct 4, 2016
Member
If some poor soul changes our layer.yaml ports, they're in trouble. Let's honor them by actually setting the chosen port in an override before calling self.trigger_bigtop() here. Something like:
self._add_override('zeppelin::server::server_port', self.dist_config.port('zeppelin'))
self._add_override('zeppelin::server::web_socket_port', self.dist_config.port('zeppelin_web'))Those should be the correct overrides, as defined in zeppelin's init.pp here:
| +def unconfigure_spark(): | ||
| + hookenv.status_set('maintenance', 'removing spark relation') | ||
| + zeppelin = Zeppelin() | ||
| + zeppelin.configure_spark(None) |
kwmonroe
Oct 4, 2016
Member
I think we should check for hadoop.ready here, and if that's true, use yarn-client as the spark master url instead of None. I'm pretty sure zeppelin can run it's internal spark interpreter in yarn-client mode without any external spark master.
FWIW, bigtop defines spark_master_url as yarn-client by default:
https://github.com/apache/bigtop/blob/master/bigtop-deploy/puppet/modules/zeppelin/manifests/init.pp#L25
johnsca
Oct 7, 2016
Contributor
If Spark isn't related to Zeppelin, there's a fairly good chance that it's not related to Hadoop, either, and thus it would fail in yarn-client mode.
Perhaps a better solution would be to remove the override entirely and just let Bigtop fall back to its default naturally.
johnsca
Oct 7, 2016
Contributor
Since the overrides are additive, removing one just leaves it as its current value, so we'll need to explicitly change it back to yarn-client.
| + Deployment and smoke test for the Apache Bigtop Zeppelin service. | ||
| + """ | ||
| + @classmethod | ||
| + def setUpClass(cls): |
kwmonroe
Oct 4, 2016
Member
We can simplify this class by removing all references to 'openjdk' and 'java' below. Do it!
johnsca
and others
added some commits
Oct 7, 2016
asfgit
closed this
in
f7d471b
Oct 8, 2016
kwmonroe
deleted the
juju-solutions:BIGTOP-2516-Zeppelin
branch
Oct 8, 2016
pushed a commit
to kulikovav/bigtop
that referenced
this pull request
Feb 8, 2017
ktsakalozos commentedAug 10, 2016
No description provided.