You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by markito <gi...@git.apache.org> on 2016/06/21 17:58:05 UTC
[GitHub] incubator-geode pull request #171: feature/geode 33
GitHub user markito opened a pull request:
https://github.com/apache/incubator-geode/pull/171
feature/geode 33
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-geode feature/GEODE-33
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-geode/pull/171.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #171
----
commit 4b6200d0ffc92c1e9fe9b1695c0f5ccd99e6c387
Author: William Markito <wm...@pivotal.io>
Date: 2016-06-09T03:30:32Z
[GEODE-33] - Adding geode-example module
Creating a simple example for replicated regions to test the proposed example model
Adding script tests
Adding server folders to gitignore
Adding gitignore and readme files
commit d2ec280df6d3feacccdb5dba4a114ef086a71717
Author: Karen Miller <km...@pivotal.io>
Date: 2016-06-15T21:34:37Z
GEODE-1531: Improve README.md for replicated example
- Improved the appearance of the markdown
- Added commands for how to kill a single server
- Added last step to run scripts/stopAll.sh so the system is
shut down when the example ends, not leaving a locator and
a server running
- Improved the prose describing what the example does
commit 21d30ab92d2a2146207186a690eb5a36d42e7059
Author: Karen Miller <km...@pivotal.io>
Date: 2016-06-15T22:30:03Z
GEODE-1525: Examples README.md should set env variable
GEODE-1523: Improve examples README.md markdown
This PR fixes addresses both GEODE-1525 and GEODE-1523, as they
both change the contents of the same file: geode-examples/README.md.
- Each example will likely use a scripts/setEnv.sh script to set
the path to gfsh. The script depends on a GEODE_HOME environment
variable, so this top level of instructions for setting up
the examples now tells the user to set a GEODE_HOME env variable
directly after the installation.
- Implement a more strict markdown that displays correctly for a
wide variety of markdown implementations.
- Put in links for the 3 references.
- Improve the prose.
commit e24a47110fb504cf4b2980305ab492e23d70a361
Author: Karen Miller <km...@pivotal.io>
Date: 2016-06-15T18:07:03Z
Merge #158 into feature/GEODE-33
GEODE-1530: geode-examples setEnv.sh script needs to export path
Without an export of the path, using the GEODE_HOME env variable,
the scripts/startAll.sh script will pick up the first gfsh in the
path that it finds.
Also updated the scripts/stopAll.sh script to run the setEnv.sh
script, so that stopAll.sh uses the correct gfsh.
commit f371995ebc0ed075342a1ae41ed8dde8189c97c3
Author: William Markito <wm...@pivotal.io>
Date: 2016-06-20T15:56:07Z
Merge #159 into feature/GEODE-33
commit d67bf627c096e025cb0f7db7af49a6cfbc30bc06
Author: William Markito <wm...@pivotal.io>
Date: 2016-06-20T15:57:16Z
Merge #160 into feature/GEODE-33
commit a31b8482a3306856f9e97459649ac38d0db709bf
Author: William Markito <wm...@pivotal.io>
Date: 2016-06-21T07:00:41Z
[GEODE-33] Fixed hardcoded path for resources on tests
- Included rat exclusion rules
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #171: feature/geode 33
Posted by gesterzhou <gi...@git.apache.org>.
Github user gesterzhou commented on the issue:
https://github.com/apache/incubator-geode/pull/171
There's no lucene in example list?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] incubator-geode pull request #171: feature/geode 33
Posted by Karen Miller <km...@pivotal.io>.
I started from scratch with a clean copy of the the GEODE-33 branch, and
ran the replicated
example. Everything worked well.
+1
On Thu, Jul 7, 2016 at 4:54 PM, markito <gi...@git.apache.org> wrote:
> Github user markito commented on a diff in the pull request:
>
>
> https://github.com/apache/incubator-geode/pull/171#discussion_r70007678
>
> --- Diff: geode-examples/replicated/README.md ---
> @@ -0,0 +1,47 @@
> +# Geode replicated region example
> +
> +This is one of the most basic examples.
> +Two servers host a replicated region.
> +The producer puts 50 entries into the replicated region. The consumer
> prints the number of entries in the region.
> +
> +## Steps
> +1. From the ```geode-examples/replicated``` directory, start the
> locator and two servers:
> +
> + $ scripts/startAll.sh
> +
> +2. Run the producer:
> +
> + $ gradle run -Pmain=Producer
> + ...
> + ...
> + INFO: Done. Inserted 50 entries.
> +
> +3. Run the consumer:
> +
> + $ gradle run -Pmain=Consumer
> + ...
> + ...
> + INFO: Done. 50 entries available on the server(s).
> +
> +4. Kill one of the servers:
> +
> + $ gfsh
> + ...
> + gfsh>connect
> + gfsh>stop server --name=server1
> + gfsh>quit
> +
> +5. Run the consumer a second time, and notice that all the entries
> are still available due to replication:
> +
> + $ gradle run -Pmain=Consumer
> + ...
> + ...
> + INFO: Done. 50 entries available on the server(s).
> +
> +6. Shutdown the system:
> +
> + $ scripts/stopAll.sh
> +
> +This example is a simple demonstration on basic APIs of Geode, as
> well how to write tests using mocks for Geode applications.
> +
> +TODO: assume jUnit4
> --- End diff --
>
> Oops, let's remove that line... Not needed anymore. Thanks.
>
>
> ---
> If your project is set up for it, you can reply to this email and have your
> reply appear on GitHub as well. If your project does not have this feature
> enabled and wishes so, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
>
[GitHub] incubator-geode pull request #171: feature/geode 33
Posted by markito <gi...@git.apache.org>.
Github user markito commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/171#discussion_r70007678
--- Diff: geode-examples/replicated/README.md ---
@@ -0,0 +1,47 @@
+# Geode replicated region example
+
+This is one of the most basic examples.
+Two servers host a replicated region.
+The producer puts 50 entries into the replicated region. The consumer prints the number of entries in the region.
+
+## Steps
+1. From the ```geode-examples/replicated``` directory, start the locator and two servers:
+
+ $ scripts/startAll.sh
+
+2. Run the producer:
+
+ $ gradle run -Pmain=Producer
+ ...
+ ...
+ INFO: Done. Inserted 50 entries.
+
+3. Run the consumer:
+
+ $ gradle run -Pmain=Consumer
+ ...
+ ...
+ INFO: Done. 50 entries available on the server(s).
+
+4. Kill one of the servers:
+
+ $ gfsh
+ ...
+ gfsh>connect
+ gfsh>stop server --name=server1
+ gfsh>quit
+
+5. Run the consumer a second time, and notice that all the entries are still available due to replication:
+
+ $ gradle run -Pmain=Consumer
+ ...
+ ...
+ INFO: Done. 50 entries available on the server(s).
+
+6. Shutdown the system:
+
+ $ scripts/stopAll.sh
+
+This example is a simple demonstration on basic APIs of Geode, as well how to write tests using mocks for Geode applications.
+
+TODO: assume jUnit4
--- End diff --
Oops, let's remove that line... Not needed anymore. Thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #171: feature/geode 33
Posted by markito <gi...@git.apache.org>.
Github user markito closed the pull request at:
https://github.com/apache/incubator-geode/pull/171
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #171: feature/geode 33
Posted by markito <gi...@git.apache.org>.
Github user markito commented on the issue:
https://github.com/apache/incubator-geode/pull/171
@jhuynh1, @gesterzhou , @nabarunnag just included the feedback items. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #171: feature/geode 33
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on the issue:
https://github.com/apache/incubator-geode/pull/171
Hi William,
Thanks for doing all of this work! This looks like a good start for examples. I have a few comments below.
1. Did you look into using the gradle application plugin, rather than writing a custom run task?
2. We'll need to verify that the gradle wrapper jar doesn't become part of the source distribution. I guess we need to do the same thing with this gradle wrapper jar that we did at the top level of the project.
3. I wonder if this ShellUtil class should be put into some common utility project for these example tests.
4. I see a problem with the ShellUtil code; it's not consuming the output of the process. Especially on windows this can cause bizarre hangs because the process will block trying to write to stdout. Also it makes it really hard to debug failures if you all you do is assert that the status is 0 but don't provide the output of the process. The code should probably include the process output in the error message if the process returns a non-zero exit.
5. 'geode-examples/*/scripts/**' should not be excluded from rat.
6. How should we integrate this with the rest of the geode build? I think these examples should probably get shipped as part of the source and binary distributions, and the the examples should be built and tested as part of precheckin.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #171: feature/geode 33
Posted by scmbuildguy <gi...@git.apache.org>.
Github user scmbuildguy commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/171#discussion_r67959530
--- Diff: geode-examples/README.md ---
@@ -0,0 +1,70 @@
+# Apache Geode examples
+
+This is the home of Apache Geode examples that are bundled with the project. Contributions<sup>[2]</sup> and corrections are welcome. Please talk to us about your suggestions at [dev@geode.incubator.apache.org](mailto:dev@geode.incubator.apache.org) or submit a [pull request](https://github.com/apache/incubator-geode/pull/new/develop).
+
+## Example requirements
+
+All examples:
+
+* Need to be testable. Use unit tests, integration tests or whatever is applicable. Tests will run through the project's CI.
+* Should be `Gradle` projects or part of existing ones. There may be exceptions here, but the community should have a consensus to accept.
+* Have to follow code format & style from Apache Geode <sup>[1]</sup> guidelines.
+* Should contain a `README.md` file with step-by-step instruction on how to set up and run the example. *Diagrams give you extra credit.*
+* Donations need to be licensed through ASL 2.0 and contributors need to file an ICLA<sup>[3]</sup>.
+
+## Structure
+
+### Installation and a Tutorial for Beginners
+
+* [How to Install](http://geode.docs.pivotal.io/docs/getting_started/installation/install_standalone.html)
+* Set a `GEODE_HOME` environment variable to point to the `bin` directory of the installation. For those that have built from source, this will be the `geode-assembly/build/install/apache-geode` directory.
--- End diff --
Seems like the language is unclear here: Set a `GEODE_HOME` environment variable to point to the `bin` directory
Pointing to 'bin' would not be correct as all the scripts would not work. I think wording should be changed to: 'root' directory of the installation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #171: feature/geode 33
Posted by nabarunnag <gi...@git.apache.org>.
Github user nabarunnag commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/171#discussion_r70006027
--- Diff: geode-examples/replicated/README.md ---
@@ -0,0 +1,47 @@
+# Geode replicated region example
+
+This is one of the most basic examples.
+Two servers host a replicated region.
+The producer puts 50 entries into the replicated region. The consumer prints the number of entries in the region.
+
+## Steps
+1. From the ```geode-examples/replicated``` directory, start the locator and two servers:
+
+ $ scripts/startAll.sh
+
+2. Run the producer:
+
+ $ gradle run -Pmain=Producer
+ ...
+ ...
+ INFO: Done. Inserted 50 entries.
+
+3. Run the consumer:
+
+ $ gradle run -Pmain=Consumer
+ ...
+ ...
+ INFO: Done. 50 entries available on the server(s).
+
+4. Kill one of the servers:
+
+ $ gfsh
+ ...
+ gfsh>connect
+ gfsh>stop server --name=server1
+ gfsh>quit
+
+5. Run the consumer a second time, and notice that all the entries are still available due to replication:
+
+ $ gradle run -Pmain=Consumer
+ ...
+ ...
+ INFO: Done. 50 entries available on the server(s).
+
+6. Shutdown the system:
+
+ $ scripts/stopAll.sh
+
+This example is a simple demonstration on basic APIs of Geode, as well how to write tests using mocks for Geode applications.
+
+TODO: assume jUnit4
--- End diff --
TODO for an assumption rather than a future task may be incorrect ???
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #171: feature/geode 33
Posted by markito <gi...@git.apache.org>.
Github user markito commented on the issue:
https://github.com/apache/incubator-geode/pull/171
@gesterzhou Not yet. We can definitely add to the list for 1.0.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode pull request #171: feature/geode 33
Posted by scmbuildguy <gi...@git.apache.org>.
Github user scmbuildguy commented on a diff in the pull request:
https://github.com/apache/incubator-geode/pull/171#discussion_r67958323
--- Diff: geode-examples/build.gradle ---
@@ -0,0 +1,51 @@
+/*
+ * 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.
+ */
+
+
+subprojects {
+
+ apply plugin:'java'
+
+ repositories {
+ maven {
+ url "https://repository.apache.org/content/repositories/snapshots/"
+ }
+ mavenCentral()
+ }
+
+ dependencies {
+ compile "org.apache.geode:geode-core:$geodeVersion"
+// testCompile "org.apache.geode:geode-junit:$geodeVersion"
+ testCompile "junit:junit:$junitVersion"
+ testCompile "org.mockito:mockito-core:$mockitocoreVersion"
+ compile "com.jayway.awaitility:awaitility:1.7.0"
--- End diff --
Add version to gradle.properties?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-geode issue #171: feature/geode 33
Posted by jhuynh1 <gi...@git.apache.org>.
Github user jhuynh1 commented on the issue:
https://github.com/apache/incubator-geode/pull/171
Some minor improvements/corrections:
1.) remove @author tag from Consumer.java
2.) The test checkIfScriptsAreExecutable runs the same line twice. I think the second line was supposed to check the stop scripts and not the start scripts
3.) In one of the tests, we probably should check that Consumer.NUM_ENTRIES is > 0. Just incase someone accidentally changes it to 0, the tests would pass but not really execute what the producer and consumer tests were expecting.
4.) preference/style : Perhaps some of the test names can be slightly more descriptive just so we know what the behavior should be.. such as changing populateWhenRegionDoesntExist to populateWhenRegionDoesNotExistShouldThrowNullPointer?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---