[GitHub] ctubbsii commented on a change in pull request #1: Provide initial README
ctubbsii commented on a change in pull request #1: Provide initial README URL: https://github.com/apache/fluo-bytes/pull/1#discussion_r132365844 ## File path: README.md ## @@ -0,0 +1,102 @@ + + +# Apache Fluo Bytes + +[![Build Status][ti]][tl] [![Apache License][li]][ll] [![Maven Central][mi]][ml] [![Javadoc][ji]][jl] + +**Apache Fluo Bytes is an API project with the goal of providing an extremely +stable library for handling bytes, suitable for use in [Apache Fluo][fluo] and +other projects' APIs.** + +## Features and Goals + +This project aims to fill a void in Java, by providing convenient objects to +represent a string of bytes and associated utility classes for situations when +a raw byte array is not appropriate. + +Specifically, it provides a `ByteSequence` interface, analogous to Java's +`CharSequence`, an immutable `Bytes` implementation analogous to Java's +`String`, and a corresponding `BytesBuilder` analogous to Java's +`StringBuilder`. + +The provided classes have appropriate methods for serialization, and proper +equals and hashCode implementations, as well as a comparator for +`ByteSequence`, so they will be suitable for use in `Set`s and as keys in +`Map`s. + +An immutable bytes implementation makes it possible to pass data between APIs +without the need for performance-killing protective copies. This benefit is +compounded if this library is used by multiple projects, as the need to make +protective copies while passing data between a project and its dependency's API +is eliminated. + +This project aims to provide a fluent and intuitive API, with support for +conversions to/from other common types, such as `ByteBuffer`, `byte[]`, and +`CharSequence`/`String`. + +This project requires at least Java 8, and supports `Stream` and functional +APIs where appropriate. + +See this [blog post][blog] for some additional background. + +## Safe for APIs + +Using an external library in a project's API poses some risks to that project, +especially if it and its dependencies depend on different versions of that +library. This project attempts to mitigate those risks, so that it can be used +safely by other projects. + +This project is made safe for reuse in other projects' APIs by adopting the +following principles: + +* Using [Semantic Versioning 2.0.0][semver] to make strong declarations about + backwards-compatibility +* Strongly avoid breaking changes (avoid major version bumps), so that projects Review comment: Ugh, I hate that new "feature" to Java 9, especially since it defaults to "false". Personally, I think all deprecations are subject to removal. I really don't want to start putting "forRemoval = true" in all my deprecation annotations just so users don't think "this isn't going away". I'd rather they'd have created a new annotation for things which are expected to remain, but should not be used by users, which generated similar compiler warnings as Deprecated. This one is really going to cause confusion with defaulting to false. A lot of them are going to be set to false simply because nobody's explicitly setting them to true. Users are going to be disappointed when they disappear. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ctubbsii commented on a change in pull request #1: Provide initial README
ctubbsii commented on a change in pull request #1: Provide initial README URL: https://github.com/apache/fluo-bytes/pull/1#discussion_r132365435 ## File path: README.md ## @@ -0,0 +1,102 @@ + + +# Apache Fluo Bytes + +[![Build Status][ti]][tl] [![Apache License][li]][ll] [![Maven Central][mi]][ml] [![Javadoc][ji]][jl] + +**Apache Fluo Bytes is an API project with the goal of providing an extremely +stable library for handling bytes, suitable for use in [Apache Fluo][fluo] and +other projects' APIs.** + +## Features and Goals + +This project aims to fill a void in Java, by providing convenient objects to +represent a string of bytes and associated utility classes for situations when +a raw byte array is not appropriate. Review comment: I did try to write more on this initially, but everything I came up with was too long and seemed to drift off topic. I boiled it down to what you see here, mentioning this library is useful when byte arrays aren't appropriate, and then immediately following that statement by some of the features that make it so, naming all of the things you list here. I don't know that I'd really want more details in the README than that. I certainly don't want to repeat a blog post, or make the README a big wall of text that provides so many details, it's hard to get the gist. But, if you have specific suggestions which can improve this, I'm happy to work it in. I, however, am near my editing limit on this subject. :) This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ctubbsii commented on a change in pull request #1: Provide initial README
ctubbsii commented on a change in pull request #1: Provide initial README URL: https://github.com/apache/fluo-bytes/pull/1#discussion_r132364598 ## File path: README.md ## @@ -0,0 +1,102 @@ + + +# Apache Fluo Bytes + +[![Build Status][ti]][tl] [![Apache License][li]][ll] [![Maven Central][mi]][ml] [![Javadoc][ji]][jl] + +**Apache Fluo Bytes is an API project with the goal of providing an extremely +stable library for handling bytes, suitable for use in [Apache Fluo][fluo] and +other projects' APIs.** + +## Features and Goals + +This project aims to fill a void in Java, by providing convenient objects to +represent a string of bytes and associated utility classes for situations when Review comment: Fixed. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] ctubbsii commented on a change in pull request #1: Provide initial README
ctubbsii commented on a change in pull request #1: Provide initial README URL: https://github.com/apache/fluo-bytes/pull/1#discussion_r132364588 ## File path: README.md ## @@ -0,0 +1,102 @@ + + +# Apache Fluo Bytes + +[![Build Status][ti]][tl] [![Apache License][li]][ll] [![Maven Central][mi]][ml] [![Javadoc][ji]][jl] + +**Apache Fluo Bytes is an API project with the goal of providing an extremely +stable library for handling bytes, suitable for use in [Apache Fluo][fluo] and Review comment: I didn't see your exact wording above, because it didn't show correctly when I saw this comment in my email, but I pushed a change which basically applies the same fix, based on your feedback above the blockquote section. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] mikewalch opened a new pull request #907: Fixes #906 - Create 'fluo get-jars' command
mikewalch opened a new pull request #907: Fixes #906 - Create 'fluo get-jars' command URL: https://github.com/apache/fluo/pull/907 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] mikewalch opened a new issue #906: Create 'fluo get-jars' command
mikewalch opened a new issue #906: Create 'fluo get-jars' command URL: https://github.com/apache/fluo/issues/906 Command will retrieve application jars store in HDFS to a local directory This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] keith-turner commented on a change in pull request #902: fixes #894 merge SimpleConfiguration Objects
keith-turner commented on a change in pull request #902: fixes #894 merge
SimpleConfiguration Objects
URL: https://github.com/apache/fluo/pull/902#discussion_r132312049
##
File path:
modules/api/src/test/java/org/apache/fluo/api/config/SimpleConfigurationTest.java
##
@@ -64,12 +64,20 @@ public void testMerge() {
diff.setProperty("set1", "value13");
diff.setProperty("set3", "value14");
+SimpleConfiguration noChange = new SimpleConfiguration(sc3);
+// check to make sure this is not being changed.
+sc3.orElse(sc1).orElse(sc2);
Review comment:
need to keep the return val like
```java
sc4 = sc3.orElse(sc1).orElse(sc2);
sc3.set(...)
//ensure sc4 did not change.
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mikewalch closed issue #897: Add high level application lifecycle overview to docs
mikewalch closed issue #897: Add high level application lifecycle overview to docs URL: https://github.com/apache/fluo/issues/897 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] mikewalch closed pull request #904: Fixes #897 added context to application docs and moved some docs from?
mikewalch closed pull request #904: Fixes #897 added context to application docs and moved some docs from? URL: https://github.com/apache/fluo/pull/904 This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] keith-turner commented on a change in pull request #1: Provide initial README
keith-turner commented on a change in pull request #1: Provide initial README URL: https://github.com/apache/fluo-bytes/pull/1#discussion_r132304408 ## File path: README.md ## @@ -0,0 +1,102 @@ + + +# Apache Fluo Bytes + +[![Build Status][ti]][tl] [![Apache License][li]][ll] [![Maven Central][mi]][ml] [![Javadoc][ji]][jl] + +**Apache Fluo Bytes is an API project with the goal of providing an extremely +stable library for handling bytes, suitable for use in [Apache Fluo][fluo] and +other projects' APIs.** + +## Features and Goals + +This project aims to fill a void in Java, by providing convenient objects to +represent a string of bytes and associated utility classes for situations when Review comment: could say sequence of bytes to since in java String is so strongly associated with char This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] keith-turner commented on a change in pull request #1: Provide initial README
keith-turner commented on a change in pull request #1: Provide initial README URL: https://github.com/apache/fluo-bytes/pull/1#discussion_r132304118 ## File path: README.md ## @@ -0,0 +1,102 @@ + + +# Apache Fluo Bytes + +[![Build Status][ti]][tl] [![Apache License][li]][ll] [![Maven Central][mi]][ml] [![Javadoc][ji]][jl] + +**Apache Fluo Bytes is an API project with the goal of providing an extremely +stable library for handling bytes, suitable for use in [Apache Fluo][fluo] and Review comment: A few things I found unclear. * What is an `API project`? * What does `stable library` mean? Below is my attempt to clear those things up. My main goal was to state its a library with a stable API. ``` Apache Fluo Bytes is a project with the goal of providing a bytes library with an extremely stable API that is suitable for use in other projects' APIs. ``` This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] keith-turner commented on a change in pull request #1: Provide initial README
keith-turner commented on a change in pull request #1: Provide initial README URL: https://github.com/apache/fluo-bytes/pull/1#discussion_r132306642 ## File path: README.md ## @@ -0,0 +1,102 @@ + + +# Apache Fluo Bytes + +[![Build Status][ti]][tl] [![Apache License][li]][ll] [![Maven Central][mi]][ml] [![Javadoc][ji]][jl] + +**Apache Fluo Bytes is an API project with the goal of providing an extremely +stable library for handling bytes, suitable for use in [Apache Fluo][fluo] and +other projects' APIs.** + +## Features and Goals + +This project aims to fill a void in Java, by providing convenient objects to +represent a string of bytes and associated utility classes for situations when +a raw byte array is not appropriate. + +Specifically, it provides a `ByteSequence` interface, analogous to Java's +`CharSequence`, an immutable `Bytes` implementation analogous to Java's +`String`, and a corresponding `BytesBuilder` analogous to Java's +`StringBuilder`. + +The provided classes have appropriate methods for serialization, and proper +equals and hashCode implementations, as well as a comparator for +`ByteSequence`, so they will be suitable for use in `Set`s and as keys in +`Map`s. + +An immutable bytes implementation makes it possible to pass data between APIs +without the need for performance-killing protective copies. This benefit is +compounded if this library is used by multiple projects, as the need to make +protective copies while passing data between a project and its dependency's API +is eliminated. + +This project aims to provide a fluent and intuitive API, with support for +conversions to/from other common types, such as `ByteBuffer`, `byte[]`, and +`CharSequence`/`String`. + +This project requires at least Java 8, and supports `Stream` and functional +APIs where appropriate. + +See this [blog post][blog] for some additional background. + +## Safe for APIs + +Using an external library in a project's API poses some risks to that project, +especially if it and its dependencies depend on different versions of that +library. This project attempts to mitigate those risks, so that it can be used +safely by other projects. + +This project is made safe for reuse in other projects' APIs by adopting the +following principles: + +* Using [Semantic Versioning 2.0.0][semver] to make strong declarations about + backwards-compatibility +* Strongly avoid breaking changes (avoid major version bumps), so that projects Review comment: It will be so nice in Java 9 when the deprecated annotation has the forRemoval parameter. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] mikewalch commented on a change in pull request #904: Fixes #897 added context to application docs and moved some docs from?
mikewalch commented on a change in pull request #904: Fixes #897 added context to application docs and moved some docs from? URL: https://github.com/apache/fluo/pull/904#discussion_r132262567 ## File path: docs/applications.md ## @@ -140,12 +135,123 @@ To create an observer, follow these steps: 3. Build a jar containing these classes and include this jar in the `lib/` directory of your Fluo application. -4. Configure your Fluo instance to use this observer provider by modifying the Application section of -[fluo-app.properties]. -5. Initialize Fluo. During initialization Fluo will obtain the observed columns from the -ObserverProvider and persist the columns in Zookeeper. These columns persisted in Zookeeper -are used by transactions to know when to trigger observers. -6. Start your Fluo instance so that your Fluo workers load the new observer. +4. Configure your Fluo application to use this observer provider by modifying the Application section of +[fluo-app.properties]. Set `fluo.observer.provider` to the observer provider class name. +5. Initialize your Fluo application as described in the next section. During initialization Fluo +will obtain the observed columns from the ObserverProvider and persist the columns in Zookeeper. +These columns persisted in Zookeeper are used by transactions to know when to trigger observers. + +## Initializing a Fluo Application + +Before a Fluo Application can run, it must be initiaized. Below is an overview of what +initialization does and some of the properties that may be set for initialization. + + * **Initialize ZooKeeper** : Each application has its own area in ZooKeeper used for configuration, + Oracle state, and worker coordination. All properties, except `fluo.connections.*`, are copied + into ZooKeeper. For example, if `fluo.worker.num.threads=128` was set, then when a worker process + starts it will read this from ZooKeeper. + * **Copy Observer jars to DFS** : Fluo workers processes need the jars containing observers. These + are provided in one of the following ways. + * Set the property `fluo.observer.init.dir` to a local directory containing observer jars. The + jars in this directory are copied to DFS under `/`. When a worker is + started, the jars are pulled from DFS and added to its classpath. + * Set the property `fluo.observer.jars.url` to a directory in DFS containing observer jars. No + copying is done. When a worker is started, the jars are pulled from this location and added to + its classpath. + * Do not set any of the properties above and have the mechanism that starts the worker process + add the needed jars to the classpath. + * **Create Accumulo table** : Each Fluo application creates and configures an Accumulo table. The + `fluo.accumulo.*` properties determine which Accumulo instance is used. For performance reasons, + Fluo runs its own code in Accumulo tablet servers. Fluo attempts to copy Fluo jars into DFS and + configure Accumulo to use them. Fluo first checks the property `fluo.accumulo.jars` and if set, + copies the jars listed there. If that property is not set, then Fluo looks on the classpath to + find jars. Jars are copied to a location under `/`. + +Below are the steps to initialize an application from the command line. It is also possible to +initialize an application using Fluo's Java API. + +1. Create a copy of [fluo-app.properties] for your Fluo application. + +cp $FLUO_HOME/conf/fluo-app.properties /path/to/myapp/fluo-app.properties + +2. Edit your copy of [fluo-app.properties] and make sure to set the following: + +* Class name of your ObserverProvider +* Paths to your Fluo observer jars +* Accumulo configuration +* DFS configuration + + When configuring the observer section of fluo-app.properties, you can configure your instance for the + [phrasecount] application if you have not created your own application. See the [phrasecount] + example for instructions. You can also choose not to configure any observers but your workers will + be idle when started. + +3. Run the command below to initialize your Fluo application. Change `myapp` to your application name: + +fluo init myapp /path/to/myapp/fluo-app.properties + + A Fluo application only needs to be initialized once. After initialization, the Fluo application + name is used to start/stop the application and scan the Fluo table. + +4. Run `fluo list` which connects to Fluo and lists applications to verify initialization. + +5. Run `fluo config myapp` to see what configuration is stored in ZooKeeper. + +## Starting your Fluo application + +Follow the instructions below to start a Fluo application which contains an oracle and multiple workers. + +1. Configure [fluo-env.sh] and [fluo-conn.properties] if you have not already. Review comment: No link at bottom of page for fluo-env.sh
[GitHub] mikewalch commented on a change in pull request #904: Fixes #897 added context to application docs and moved some docs from?
mikewalch commented on a change in pull request #904: Fixes #897 added context to application docs and moved some docs from? URL: https://github.com/apache/fluo/pull/904#discussion_r132262720 ## File path: docs/applications.md ## @@ -140,12 +135,123 @@ To create an observer, follow these steps: 3. Build a jar containing these classes and include this jar in the `lib/` directory of your Fluo application. -4. Configure your Fluo instance to use this observer provider by modifying the Application section of -[fluo-app.properties]. -5. Initialize Fluo. During initialization Fluo will obtain the observed columns from the -ObserverProvider and persist the columns in Zookeeper. These columns persisted in Zookeeper -are used by transactions to know when to trigger observers. -6. Start your Fluo instance so that your Fluo workers load the new observer. +4. Configure your Fluo application to use this observer provider by modifying the Application section of +[fluo-app.properties]. Set `fluo.observer.provider` to the observer provider class name. +5. Initialize your Fluo application as described in the next section. During initialization Fluo +will obtain the observed columns from the ObserverProvider and persist the columns in Zookeeper. +These columns persisted in Zookeeper are used by transactions to know when to trigger observers. + +## Initializing a Fluo Application + +Before a Fluo Application can run, it must be initiaized. Below is an overview of what +initialization does and some of the properties that may be set for initialization. + + * **Initialize ZooKeeper** : Each application has its own area in ZooKeeper used for configuration, + Oracle state, and worker coordination. All properties, except `fluo.connections.*`, are copied + into ZooKeeper. For example, if `fluo.worker.num.threads=128` was set, then when a worker process + starts it will read this from ZooKeeper. + * **Copy Observer jars to DFS** : Fluo workers processes need the jars containing observers. These + are provided in one of the following ways. + * Set the property `fluo.observer.init.dir` to a local directory containing observer jars. The + jars in this directory are copied to DFS under `/`. When a worker is + started, the jars are pulled from DFS and added to its classpath. + * Set the property `fluo.observer.jars.url` to a directory in DFS containing observer jars. No + copying is done. When a worker is started, the jars are pulled from this location and added to + its classpath. + * Do not set any of the properties above and have the mechanism that starts the worker process + add the needed jars to the classpath. + * **Create Accumulo table** : Each Fluo application creates and configures an Accumulo table. The + `fluo.accumulo.*` properties determine which Accumulo instance is used. For performance reasons, + Fluo runs its own code in Accumulo tablet servers. Fluo attempts to copy Fluo jars into DFS and + configure Accumulo to use them. Fluo first checks the property `fluo.accumulo.jars` and if set, + copies the jars listed there. If that property is not set, then Fluo looks on the classpath to + find jars. Jars are copied to a location under `/`. + +Below are the steps to initialize an application from the command line. It is also possible to +initialize an application using Fluo's Java API. + +1. Create a copy of [fluo-app.properties] for your Fluo application. + +cp $FLUO_HOME/conf/fluo-app.properties /path/to/myapp/fluo-app.properties + +2. Edit your copy of [fluo-app.properties] and make sure to set the following: + +* Class name of your ObserverProvider +* Paths to your Fluo observer jars +* Accumulo configuration +* DFS configuration + + When configuring the observer section of fluo-app.properties, you can configure your instance for the + [phrasecount] application if you have not created your own application. See the [phrasecount] Review comment: No link for phrasecount This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] mikewalch commented on a change in pull request #904: Fixes #897 added context to application docs and moved some docs from?
mikewalch commented on a change in pull request #904: Fixes #897 added context to application docs and moved some docs from? URL: https://github.com/apache/fluo/pull/904#discussion_r132269526 ## File path: docs/applications.md ## @@ -17,8 +17,18 @@ limitations under the License. # Fluo Applications -Once you have Fluo installed and running on your cluster, you can now run Fluo applications which -consist of clients and observers. +Once you have Fluo installed and running on your cluster, you can run Fluo applications consisting +of clients and observers. A client is a user started process that uses the Fluo API. An Observer +is user provided code run by Fluo Workers in response to notifications. Notifications are set by Review comment: Could link to architecture.md here if users want to better understand components of an application. This text could be incorporated into architecture.md to improve it. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] keith-turner commented on a change in pull request #902: fixes #894 merge SimpleConfiguration Objects
keith-turner commented on a change in pull request #902: fixes #894 merge
SimpleConfiguration Objects
URL: https://github.com/apache/fluo/pull/902#discussion_r132267259
##
File path:
modules/api/src/main/java/org/apache/fluo/api/config/SimpleConfiguration.java
##
@@ -250,6 +250,43 @@ public SimpleConfiguration subset(String prefix) {
return new SimpleConfiguration(internalConfig.subset(prefix));
}
+ /**
+ * @param fallback SimpleConfiguration to join together
+ * @return a new simple configuration that contains all of the current
properties from this plus
+ * the properties from fallback that are not present in this.
+ *
+ * @since 1.2.0
+ */
+ public SimpleConfiguration orElse(SimpleConfiguration fallback) {
+for (Map.Entry entry : fallback.toMap().entrySet()) {
Review comment:
This implementation modifies this, but the javadoc says returns a new. I
think it I would prefer returning a copy. May be able to do
`SimpleConfiguration copy = new SimpleConfiguration(this)` and then use `copy`
instead of `this` in method.
Would be nice to have a test that verifies following :
```java
sc1 = ...
sc2 = ...
sc3 = sc1.orElse(sc2)
//set something on sc1 and sc2
// verify that sc3 did not change
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] ctubbsii commented on a change in pull request #905: Add Accumulo API checks to checkstyle rules
ctubbsii commented on a change in pull request #905: Add Accumulo API checks to checkstyle rules URL: https://github.com/apache/fluo/pull/905#discussion_r132264201 ## File path: src/main/resources/org/apache/fluo/resources/java-checkstyle.xml ## @@ -170,5 +170,11 @@ + Review comment: I copied from the Fluo pom, but did not do any additional testing. I did, however, manually inspect to ensure I understood the expression and that it made sense, as a sanity check for what already existed. The only thing I changed was to make `\.` become `[.]`, because I think that construction is more clear (escapes are hard to reason about). This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] keith-turner commented on a change in pull request #904: Fixes #897 added context to application docs and moved some docs from?
keith-turner commented on a change in pull request #904: Fixes #897 added
context to application docs and moved some docs from?
URL: https://github.com/apache/fluo/pull/904#discussion_r132261694
##
File path: docs/applications.md
##
@@ -70,31 +80,6 @@ FluoClient client = FluoFactory.newClient(config)
It may help to reference the [API javadocs][API] while you are learning the
Fluo API.
-## Running application code
-
-The `fluo exec {arguments}` provides an easy way to execute
application code. It
-will execute a class with a main method if a jar containing the class is
placed in the lib directory
-of the application. When the class is run, Fluo classes and dependencies will
be on the classpath.
-The `fluo exec` command can inject the applications configuration if the class
is written in the
-following way. Defining the injection point is optional.
-
-```java
-import javax.inject.Inject;
-
-public class AppCommand {
-
- //when run with fluo exec command, the applications configuration will be
injected
- @Inject
- private static FluoConfiguration fluoConfig;
-
- public static void main(String[] args) throws Exception {
-try(FluoClient fluoClient = FluoFactory.newClient(fluoConfig)) {
- //do stuff with Fluo
-}
- }
-}
-```
-
## Creating a Fluo observer
To create an observer, follow these steps:
Review comment:
I didn't understand why you wanted to make this change and didn't change
anything.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] keith-turner commented on a change in pull request #904: Fixes #897 added context to application docs and moved some docs from?
keith-turner commented on a change in pull request #904: Fixes #897 added
context to application docs and moved some docs from?
URL: https://github.com/apache/fluo/pull/904#discussion_r132261218
##
File path: docs/applications.md
##
@@ -70,31 +80,6 @@ FluoClient client = FluoFactory.newClient(config)
It may help to reference the [API javadocs][API] while you are learning the
Fluo API.
-## Running application code
-
-The `fluo exec {arguments}` provides an easy way to execute
application code. It
-will execute a class with a main method if a jar containing the class is
placed in the lib directory
-of the application. When the class is run, Fluo classes and dependencies will
be on the classpath.
-The `fluo exec` command can inject the applications configuration if the class
is written in the
-following way. Defining the injection point is optional.
-
-```java
-import javax.inject.Inject;
-
-public class AppCommand {
-
- //when run with fluo exec command, the applications configuration will be
injected
- @Inject
- private static FluoConfiguration fluoConfig;
-
- public static void main(String[] args) throws Exception {
-try(FluoClient fluoClient = FluoFactory.newClient(fluoConfig)) {
- //do stuff with Fluo
-}
- }
-}
-```
-
## Creating a Fluo observer
Review comment:
Based on this comment I removed some redundant info at the end of this
section. Is that what you were thinking of?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mikewalch commented on a change in pull request #899: fixes #893 added ability to secure zookeeper
mikewalch commented on a change in pull request #899: fixes #893 added ability
to secure zookeeper
URL: https://github.com/apache/fluo/pull/899#discussion_r132257442
##
File path:
modules/integration/src/test/java/org/apache/fluo/integration/impl/ZKSecretIT.java
##
@@ -0,0 +1,143 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
contributor license
+ * agreements. See the NOTICE file distributed with this work for additional
information regarding
+ * copyright ownership. The ASF licenses this file to you under the Apache
License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express
+ * or implied. See the License for the specific language governing permissions
and limitations under
+ * the License.
+ */
+
+package org.apache.fluo.integration.impl;
+
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
+import java.util.concurrent.TimeUnit;
+
+import com.google.common.util.concurrent.Uninterruptibles;
+import org.apache.fluo.accumulo.util.ZookeeperPath;
+import org.apache.fluo.api.client.FluoClient;
+import org.apache.fluo.api.client.FluoFactory;
+import org.apache.fluo.api.client.Snapshot;
+import org.apache.fluo.api.client.Transaction;
+import org.apache.fluo.api.config.FluoConfiguration;
+import org.apache.fluo.api.data.Column;
+import org.apache.fluo.api.observer.ObserverProvider;
+import org.apache.fluo.integration.ITBaseMini;
+import org.apache.zookeeper.KeeperException.NoAuthException;
+import org.apache.zookeeper.ZooKeeper;
+import org.junit.Assert;
+import org.junit.Test;
+
+import static org.apache.fluo.api.observer.Observer.NotificationType.STRONG;
+
+public class ZKSecretIT extends ITBaseMini {
+
+ public static class MyObserverProvider implements ObserverProvider {
+
+@Override
+public void provide(Registry or, Context ctx) {
+ or.forColumn(new Column("edge", "forward"), STRONG).useObserver((tx,
row, col) -> {
+tx.set(tx.get(row, col), new Column("edge", "reverese"), row);
+ });
+}
+
+ }
+
+ @Override
+ protected void setConfig(FluoConfiguration config) {
+config.setZookeeperSecret("are3");
+config.setObserverProvider(MyObserverProvider.class);
+ }
+
+ private ZooKeeper getZookeeper() throws IOException {
+ZooKeeper zk = new ZooKeeper(config.getAppZookeepers(), 3, null);
+
+long start = System.currentTimeMillis();
+while (!zk.getState().isConnected() && System.currentTimeMillis() - start
< 3) {
+ Uninterruptibles.sleepUninterruptibly(10, TimeUnit.MILLISECONDS);
+}
+
+return zk;
+ }
+
+ @Test
+ public void testZKAcls() throws Exception {
+
+// verify basic functionality works when password is set in ZK
+try (FluoClient client =
FluoFactory.newClient(miniFluo.getClientConfiguration())) {
Review comment:
What happens if a user a passes in an incorrect Zookeeper secret? It would
be good to make sure there is a good exception or error message for this case.
A unit test could also be written to verify this behavior.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] mikewalch commented on a change in pull request #902: fixes #894 merge SimpleConfiguration Objects
mikewalch commented on a change in pull request #902: fixes #894 merge
SimpleConfiguration Objects
URL: https://github.com/apache/fluo/pull/902#discussion_r132228853
##
File path:
modules/api/src/main/java/org/apache/fluo/api/config/SimpleConfiguration.java
##
@@ -250,6 +250,40 @@ public SimpleConfiguration subset(String prefix) {
return new SimpleConfiguration(internalConfig.subset(prefix));
}
+ /**
+ * @param fallback SimpleConfiguration to join together
+ * @return a new simple configuration that contains all of the current
properties from this plus
+ * the properties from fallback that are not present in this.
+ *
+ * @since 1.2.0
+ */
+ public SimpleConfiguration orElse(SimpleConfiguration fallback) {
+for (Map.Entry entry : fallback.toMap().entrySet()) {
+ if (!this.containsKey(entry.getKey())) {
+this.setProperty(entry.getKey(), entry.getValue());
+ }
+}
+return this;
+ }
+
+ @Override
+ public int hashCode() {
+return Objects.hashCode(this.toString());
+ }
+
+ @Override
+ public boolean equals(Object o) {
+if (o == this) {
+ return true;
+}
+
+if (o instanceof SimpleConfiguration) {
+ SimpleConfiguration sc = (SimpleConfiguration) o;
+ return this.toString().equals(sc.toString());
Review comment:
I am not sure about checking for equality by comparing toString(). It
relies on toString() method to be very consistent. Can properties be printed in
a different order? If so, comparing two identical configuration objects could
return false. For this comparison, you could convert each simple configuration
to map, verify that the maps contain same number of entries, and verify that
all properties & values in on sc matches the other.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
