No. I had a test in RegionConfigTest to make sure the xml stays the same.
[ Full content available at: https://github.com/apache/geode/pull/2991 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I feel that we're adding unnecessary complexity. Why are we building
oddKeyFilter, if the only thing we're going to do with it is add each element
to oddKeyArgs? Why not just build the latter and throw out the former?
[ Full content available at: https://github.com/apache/geode-native/pull/409
Did u run it?
[ Full content available at: https://github.com/apache/geode-native/pull/409 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
It wouldn't be what's in the README. The print from line 75 is missing, and I
don't see where lines 21 and on (in the README) would come from.
`Console.WriteLine("Result count of: " + res.Count)`
[ Full content available at: https://github.com/apache/geode-native/pull/409 ]
This message was
quite certain that was generated output and then added to the README...
[ Full content available at: https://github.com/apache/geode-native/pull/409 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Looks like the last change wasn't checked in. New commit forthcoming.
[ Full content available at: https://github.com/apache/geode-native/pull/409 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Please update README.md with the correct output:
```
C:\Program
Files\nativeclient\examples\build\dotnet\FunctionExecutionCs\Debug>FunctionExecutionCs.exe
Storing id and username in the region
Getting the user info from the region
rtimmons = Robert Timmons
scharles = Sylvia Charles
Result count
* Do not create txEntryState for entries with tombstones during set operations
- keySet, values, etc.
* Only create txEntryState for tombstones during get operation.
Co-authored-by: lgallinat
[ Full content available at: https://github.com/apache/geode/pull/3008 ]
This message was relayed
Not sure if we care about lack of carriage returns here or not, but I just
noticed we are missing one here.
[ Full content available at: https://github.com/apache/geode-native/pull/293 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Should use of Rat be off by default? I'm assuming the `OFF` here is the
default but my cmake is rusty.
[ Full content available at: https://github.com/apache/geode-native/pull/293 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
resultList is not "really" being used. I don't think it, or the following for
loop should be in the example if we're just going to print the number of
elements in the result set.
[ Full content available at: https://github.com/apache/geode-native/pull/409 ]
This message was relayed via
Maybe this is more stylistic, but you've probably noticed that coming from the
Go-style I prefer ifs to be flat and not hierarchical, as much as possible. I'm
ok with either way, but I've noticed that in this class we now have a
dichotomy, i.e. we're doing both flat ifs and hierarchical ifs
I thought I added a helper function in ExpirationAttributes.java to do this?
Should it not be under that domain?
[ Full content available at: https://github.com/apache/geode/pull/2991 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
for this change. Assuming it doesn't affect the schema?
[ Full content available at: https://github.com/apache/geode/pull/2991 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I think I grouped these four into one global `if (regionAttributes != null)`
because they all deal with expiration attributes
[ Full content available at: https://github.com/apache/geode/pull/2991 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
The idea was to show what someone might do with the results... we don't have
anywhere to send `resultList` off to for further processing and didn't add an
extraneous comment about it... thought it was implied...
[ Full content available at: https://github.com/apache/geode-native/pull/409 ]
This
Good catch igodwin. The updated code for Program.cs now prints out the returned
items as well as the count of returned items. Also, as you point out,
resultList was not used and has been removed.
[ Full content available at: https://github.com/apache/geode-native/pull/409 ]
This message was
[ pull request closed by WireBaron ]
[ Full content available at: https://github.com/apache/geode/pull/3007 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Right -- but I'm wondering if it's worth just grouping all of them into one
global `if (regionAttributes != null)` in order to be consistent.
[ Full content available at: https://github.com/apache/geode/pull/2991 ]
This message was relayed via gitbox.apache.org for
[ pull request closed by balesh2 ]
[ Full content available at: https://github.com/apache/geode-benchmarks/pull/25
]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
balesh2 closed pull request #25: GEODE-6148: improve performance of Prepopulate
URL: https://github.com/apache/geode-benchmarks/pull/25
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
[ pull request closed by jinmeiliao ]
[ Full content available at: https://github.com/apache/geode/pull/2991 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I really like using a real object rather than power mock here. But I think the
issue is that this we apparently support multiple fixed id sizes - I see there
is a byte and short test above. I think the code probably may work differently
if the id can fit in an integer. So we probably need to
I will open a new Jira ticket to add the test coverage. I need to find out how
to construct some of the Objects -- seems it will take some time to cover all
the cases.
[ Full content available at: https://github.com/apache/geode/pull/2993 ]
This message was relayed via gitbox.apache.org for
- a .md file was deleted from the repo, but not removed from the install list
[ Full content available at: https://github.com/apache/geode-native/pull/422 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ pull request closed by pdxcodemonkey ]
[ Full content available at: https://github.com/apache/geode-native/pull/422 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I am wondering should we also have a similar concept in our config object to
indicate NONE. If the update case, if user pass in null, that means I want to
leave the existing configuration alone, if they pass in NONE, that means I want
to wipe out the previous configuration.
[ Full content
why we don't care about algorithm anymore here?
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I wonder if this is really worthwhile. This function is going to be so hard to
maintain and test. I thought we would completely get rid of this factory
classes, and just construct the config object right in the command and set the
values as we validate each group of parameters. We do not want
EvictionAttributes is already Serializable
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
The Travis failure is being fixed by
https://github.com/apache/geode-native/pull/421
[ Full content available at: https://github.com/apache/geode-native/pull/409 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I don't think the output looks like this, anymore. Line 75 in Program.cs just
prints the number of elements in the result set, nothing more.
[ Full content available at: https://github.com/apache/geode-native/pull/409 ]
This message was relayed via gitbox.apache.org for
I don't think the output looks like this, anymore. Line 75 just prints the
number of elements in the result set, nothing more.
[ Full content available at: https://github.com/apache/geode-native/pull/409 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
* all scripts now use an AWS profile named `geode-benchmarks`.
* Add script to build AMI.
Authored-by: Sean Goller
[ Full content available at: https://github.com/apache/geode-benchmarks/pull/29
]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
smgoller opened a new pull request #29: Use a dedicated separate profile to
work with AWS.
URL: https://github.com/apache/geode-benchmarks/pull/29
* all scripts now use an AWS profile named `geode-benchmarks`.
* Add script to build AMI.
Authored-by: Sean Goller
Maybe we should take that line out then, so that people don't wonder why it is
never printed.
[ Full content available at: https://github.com/apache/geode-native/pull/409 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Paired on this with @jinmeiliao, we have a resolution.
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
We have enough end-to-end test coverage such that we don't need a unit test
here. However, we can backfill one.
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
We have enough end-to-end test coverage such that we don't need a unit test
here.
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
The reason we had the factory class in the first place is to be able to
test-drive in isolation. The original unit test was quite easy, it's just that
with all these parameters it's a little harder to read. But I'm not sure if I
agree that it would be easier if this was all in
Sure, we could change the name.
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Sure, that's a refactor we could do.
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
There's nothing in the XML POJO that indicates that, and we didn't want to
touch it. But yes, that would be a better abstraction.
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Sure, will fix.
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
We had a test case where the algorithm was provided but the intended behavior
was no eviction (since action was none).
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Sure
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
README updated.
[ Full content available at: https://github.com/apache/geode-native/pull/409 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I think you missed one more thing about the behavior -- is that if
"ifNotExists" is set, throw an EntityExistsException with an "OK" status, so
that the caller can check that status and not fail. So it's a little more
subtle. I think we can figure out a better name for sure.
[ Full content
I think you missed one thing about the behavior -- is that if "ifNotExists" is
set, throw an EntityExistsException with an "OK" status, so that the caller can
check that status and not fail. So it's a little more subtle. I think we can
figure out a better name for sure.
[ Full content
I think you missed one more thing about the behavior -- is that if
"ifNotExists" is set, throw an EntityExistsException with an "OK" status, so
that the caller can check that status and not fail. So it's a little more
subtle. I think we can figure out a better name for sure. A boolean return
The reason we had the factory class in the first place is to be able to
test-drive in isolation. The original unit test was quite easy, it's just that
with all these parameters it's a little harder to read now. But I'm not sure if
I agree that it would be easier if this was all in
@mcmellawatt can you please re-review and lift your block?
[ Full content available at: https://github.com/apache/geode-native/pull/293 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
The reason we had the factory class in the first place is to be able to
test-drive in isolation. The original unit test was quite easy, it's just that
with all these parameters it's a little harder to read now. But I'm not sure if
I agree that it would be easier if this was all in
The reason we had the factory class in the first place is to be able to
test-drive in isolation. The original unit test was quite easy, it's just that
with all these parameters it's a little harder to read now. But I'm not sure if
I agree that it would be easier if this was all in
The reason we had the factory class in the first place is to be able to
test-drive in isolation. The original unit test was quite easy, it's just that
with all these parameters it's a little harder to read now. But I'm not sure if
I agree that it would be easier if this was all in
Yes, and the plan is to get rid of the other one when we refactor
AlterRegionCommand and remove RegionFunctionArgs
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I think we can refactor this condition a bit.
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Sure -- this function was essentially a copy-pasta.
[ Full content available at: https://github.com/apache/geode/pull/2998 ]
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/2945 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
59 matches
Mail list logo
