Thank you for submitting a contribution to Apache Geode.
@jhuynh1 @Bill
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the
commit
…taDataOp could end up with NPE (#2952)"
Test6149
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR?
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the
commit message?
- [ ] Has
Thumbs up! Already approved.
[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ pull request closed by dschneider-pivotal ]
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the
commit message?
- [ ] Has
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [X] Is there a JIRA ticket associated with this PR? Is it referenced in the
commit message?
- [X] Has
WireBaron opened a new pull request #18: GEODE-6163: fix the argument handling
in the benchmark run analyzer
URL: https://github.com/apache/geode-benchmarks/pull/18
This is an automated message from the Apache Git Service.
[ Full content available at: https://github.com/apache/geode-benchmarks/pull/18
]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
@kirklund
No user facing public API packages are being altered in this PR. I think if I
had to put my sticking point somewhere it would be on keeping module and
artifact names consistent. I am less sticky on the package names inside the
modules being consistent with the module name. There is
The difference between `org.apache.geode.cache.query` and
`org.apache.geode.query` is fairly small but they are non-internal User APIs.
So for fixing modularization I would lean away from changing the User packages
-- or do so as little as possible. The impression I got on the dev-list was
@kirklund
> We can't use org.apache.geode.cache.query.cq inside geode-cq? Or is that just
> convention?
Since package`org.apache.geode.cache.query.cq` is not in the `geode-core`
module we could use that package in `geode-cq` module. It is convention to keep
package, module and artifact names
@kirklund
> We can't use org.apache.geode.cache.query.cq inside geode-cq? Or is that just
> convention?
Since package`org.apache.geode.cache.query.cq` is not in the `geode-core`
module we could use that package in `geode-cq` module. It is convention to keep
package, module and artifact names
@aditya87
[ Full content available at: https://github.com/apache/geode/pull/2967 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
…ead of InternalGfshCommand
* eventually InternalGfshCommand should be deleted and replaced with GfshCommand
Co-authored-by: Peter Tran
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have
* writePdx checks whether the object passed is an internal object as
the first step and returns false if that is the case.
* This prevents unnecessary allocation of memory and putting strain on
the GC machinery.
Thank you for submitting a contribution to Apache Geode.
In order
- Changed tests that had been expecting member specific configurations to not
be persisted
Co-authored-by: Jens Deppe
Co-authored-by: Kenneth Howe
Co-authored-by: Aditya Anchuri
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we
nabarunnag closed pull request #16: GEODE-6084: Refactored the benchmark code
as per review.
URL: https://github.com/apache/geode-benchmarks/pull/16
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
[ pull request closed by nabarunnag ]
[ Full content available at: https://github.com/apache/geode-benchmarks/pull/16
]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the
commit message?
- [ ] Has
Hello all.
I wanted to see if the dependencies that Nebula moved / changed the scope /
removed breaks under precheckin. No need to review, as this should be split
into multiple PRs, and the POM changes might warrant discussion.
[ Full content available at:
[ pull request closed by jinmeiliao ]
[ Full content available at: https://github.com/apache/geode/pull/2962 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I forgot geode-cq is a module but geode-query is not [edited]. It still seems
like cq is part of query(ing) though. We can't use
org.apache.geode.cache.query.cq inside geode-cq? Or is that just convention?
Would we ever create geode-query to contain org.apache.geode.cache.query and
then
I forgot geode-cq is a module but geode-query is not [edited]. It still seems
like cq is part of query(ing) though. We can't use org.apache.geode.query.cq
inside geode-cq? Or is that just convention? Would we ever create geode-query
to contain org.apache.geode.query and then geode-cq adds cq
I forgot about geode-cq as a module. It still seems like cq is part of
query(ing) though. We can't use org.apache.geode.query.cq inside geode-cq? Or
is that just convention? Would we ever create geode-query to contain
org.apache.geode.query and then geode-cq adds cq onto that?
I'm thinking
Okay. Just wondering. I see that the regular "GfshCommand" class also has
access to the current Gfsh instance, though. So I'm sure there's something here
which I'm not understanding, we can discuss that another time :).
[ Full content available at: https://github.com/apache/geode/pull/2962 ]
Oh, that statement should have a clause: with the same test coverage, junit is
preferred than integration test, integration test is preferred than dunit. Like
you don't need a dunit test to test a invalid command option.
[ Full content available at: https://github.com/apache/geode/pull/2962 ]
Oh, that statement should have a clause: with the same test coverage, junit is
preferred than integration test, integration test is preferred than dunit
[ Full content available at: https://github.com/apache/geode/pull/2962 ]
This message was relayed via gitbox.apache.org for
I am convinced that we don't need a DUnit test for this one.
However, I'm not sure I agree with the general principle of not preferring
integration tests? Especially in a distributed system like Geode, I feel that
the greater robustness and less mocky nature of integration tests trumps the
I am convinced that we don't need a DUnit test for this one.
However, I'm not sure I agree with the general principle of not having
integration tests? Especially in a distributed system like Geode, I feel that
the greater robustness and less mocky nature of integration tests trumps the
ease
Okay. Just wondering.
[ Full content available at: https://github.com/apache/geode/pull/2962 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ pull request closed by mcmellawatt ]
[ Full content available at: https://github.com/apache/geode/pull/2959 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ pull request closed by gesterzhou ]
[ Full content available at: https://github.com/apache/geode/pull/2952 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
These commands are different from other commands that it doesn't go through
that usual workflow. This is simply gather the options and start a jConsole
process. Our future rest service would have nothing to do with it. It needs
InternalGfshCommand since it needs reference to the gfsh instance
for the changes we are making, a junit test should do. An end to end test would
need to fire up additional process, hard to control in a test environment.
We should always prefer junit test over integration test and integration over
dunit tests. Use dunit tests only when necessary.
[ Full
I put a comment on your JIRA - it seems that ClientCacheFactory actually
doesn't require spring shell - just the way you are creating a client, thanks
to some crazy if checks.
That said, this is still a mess. Seems like we should remove all of the
optional flags, maybe? Or actually refactor
Shouldn't this be SingleGfshCommand or GfshCommand?
[ Full content available at: https://github.com/apache/geode/pull/2962 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Curious, do we not have more end-to-end test coverage for this command?
[ Full content available at: https://github.com/apache/geode/pull/2962 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
no need to do this. all locators/servers started by the rule will be shut down
by the rule.
[ Full content available at: https://github.com/apache/geode/pull/2960 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Camel case package name needs a bit weird. Does the JVMParameters class even
need to be a it's own package? seems like it shouldn't be under client/server
[ Full content available at: https://github.com/apache/geode-benchmarks/pull/16
]
This message was relayed via gitbox.apache.org for
upthewaterspout commented on a change in pull request #16: GEODE-6084:
Refactored the benchmark code as per review.
URL: https://github.com/apache/geode-benchmarks/pull/16#discussion_r239613439
##
File path:
Typo - Paramters instead of Parameters.
[ Full content available at: https://github.com/apache/geode-benchmarks/pull/16
]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
upthewaterspout commented on a change in pull request #16: GEODE-6084:
Refactored the benchmark code as per review.
URL: https://github.com/apache/geode-benchmarks/pull/16#discussion_r239613550
##
File path:
Co-authored-by: Matthew Reddington
[ Full content available at: https://github.com/apache/geode-native/pull/416 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
dunit tests timed out - I'm checking into it
[ Full content available at: https://github.com/apache/geode/pull/2956 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
The reason we elected to use a new class scoped data source class was so that
in the future if which data sources are valid changes this test will still be
comparing against an 'invalid' data source class. We originally tried to
compare it against a Managed or XAPooled data source class but ran
[ pull request closed by jinmeiliao ]
[ Full content available at: https://github.com/apache/geode/pull/2875 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Co-authored-by: Peter Tran
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the
Ah, makes sense and good to hear.
[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
@aditya87
[ Full content available at: https://github.com/apache/geode/pull/2962 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ pull request closed by pivotal-jbarrett ]
[ Full content available at: https://github.com/apache/geode-native/pull/401 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Should also mention that it goes away completely with the upcoming
`module-info.java` changes.
[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
The modules "plugin" expects it project. It is consistent with the experimental
plugin from Gradle. Eventually they will have first level support for this.
[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for
[ Full content available at: https://github.com/apache/geode/pull/2961 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
These are the minimum changes to get modules working from a client. Yes other
deps may be old but out of scope for this work.
[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Always love to see these simple sorts of cleanups.
[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Just a reminder that, by popular request, `spotless` will no longer remove
extra whitespace within a line, even if the result will no longer exceed the
80-width limit. For instance, this line's diff isn't necessary for spotless
(but I assume it was at some point during your iteration).
[ Full
I'm really not a fan of bloating out the `ext` in our project / subprojects.
Is there a reason this shouldn't just be in-lined below?
[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I don't really know what this file is documenting, but every other version here
is wildly out of date
[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ pull request closed by dschneider-pivotal ]
[ Full content available at: https://github.com/apache/geode/pull/2957 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
- Handle request data error messages for function execution on a region.
Signed-off-by: Ernest Burghardt
[ Full content available at: https://github.com/apache/geode-native/pull/415 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ pull request closed by nabarunnag ]
[ Full content available at: https://github.com/apache/geode-benchmarks/pull/17
]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
nabarunnag closed pull request #17: Fixing the warnings mentioned by LGTM.
URL: https://github.com/apache/geode-benchmarks/pull/17
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a
@jinmeiliao Please take a look and review!
[ Full content available at: https://github.com/apache/geode/pull/2960 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
- Added missing DUnit test for the command.
Signed-off-by: Ken Howe
Signed-off-by: Aditya Anchuri
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
-
@kirklund I tried to keep the package names, module names and project names
consistent. I am up for any naming really but we need consistency that we don't
have today. The rational for package `org.apache.geode.cq` is that fits with
the Maven coordinate of `org.apache.geode:geode-cq` and the
Closed. This PR has been superseded by PR#2943
[ Full content available at: https://github.com/apache/geode/pull/2926 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ pull request closed by pdxrunner ]
[ Full content available at: https://github.com/apache/geode/pull/2926 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
@kirklund I tried to keep the package names, module names and project names
consistent. I am up for any naming really but we need consistency that we don't
have today.
So you are asking for classes in `org.apache.geode.cache.query.internal.cq` to
move to
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [X] Is there a JIRA ticket associated with this PR? Is it referenced in the
commit message?
- [X] Has
[ pull request closed by pdxrunner ]
[ Full content available at: https://github.com/apache/geode/pull/2943 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ pull request closed by PurelyApplied ]
[ Full content available at: https://github.com/apache/geode/pull/2940 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
This is where the AssemblyContentIntegrationTest dumps the file it generates
(at least on my workstation). I copy this file to the src folder that it is
supposed to reference, but this file itself I usually delete so that it doesn't
get checked in. I thought I'd gitignore it so I don't have to
Why is this change needed?
[ Full content available at: https://github.com/apache/geode/pull/2875 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ pull request closed by kirklund ]
[ Full content available at: https://github.com/apache/geode/pull/2944 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
The log4j syntax uses parameterization:
```
logger.info("Starting container {} RMI Port: {}", description, jvmJmxPort);
```
[ Full content available at: https://github.com/apache/geode/pull/2938 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
+1
[ Full content available at: https://github.com/apache/geode/pull/2875 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ pull request closed by mcmellawatt ]
[ Full content available at: https://github.com/apache/geode/pull/2955 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Can we mark this with a JIRA ticket number, so we can track it?
[ Full content available at: https://github.com/apache/geode/pull/2875 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
nabarunnag opened a new pull request #17: Fixing the warnings mentioned by LGTM.
URL: https://github.com/apache/geode-benchmarks/pull/17
This is an automated message from the Apache Git Service.
To respond to the message,
I know right? This is a result of us putting `java` in a `subproject` block.
Gradle treats all directories as projects so all directories are Java!
[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for
will do
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
will do
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
apis do not change cluster config. Other than that api region destroy does what
it always has done. The only additional thing you would want to remove when
using the apis is the async queue on the cache. Of course this is already true
for the apis; if the region is the last one using the queue
No changes needed, but it would be nice if the build didn't assume that every
subdirectory is a `java` project.
[ Full content available at: https://github.com/apache/geode/pull/2915 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
My concern is that this would make the region commands aware of a specific
extension point, which makes it not extensible after all. JDBC is an extension
of the core, I guess the idea is some customer might install geode without
installing this extension. Is it right for the core to depend on
same here, you can use
```suggestion
RegionMapping mapping =
CacheElement.findElement(cacheConfig.getCustomRegionElements(), "jdbc-mapping")
```
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for
I believe you can use
```suggestion
RegionConfig regionConfig =
CacheElement.findElement(cacheConfig.getRegions, regionName)
```
[ Full content available at: https://github.com/apache/geode/pull/2950 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
89 matches
Mail list logo
