You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Mike Drob <md...@mdrob.com> on 2014/04/28 21:18:50 UTC
Review Request 20790: ACCUMULO-2742 offset history command by one
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20790/
-----------------------------------------------------------
Review request for accumulo.
Bugs: ACCUMULO-2742
https://issues.apache.org/jira/browse/ACCUMULO-2742
Repository: accumulo
Description
-------
ACCUMULO-2742 offset history command by one
The history entries returned by the history command are 0-indexed,
while the history expansion is 1-indexed. We need to offset the index
when we print it so that users can accurately use event expansion.
Diffs
-----
core/src/main/java/org/apache/accumulo/core/util/shell/commands/HistoryCommand.java 9531d903aca834ab3b70650824023061e7e788d9
core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/20790/diff/
Testing
-------
New unit test. Verified on cluster deployment.
Thanks,
Mike Drob
Re: Review Request 20790: ACCUMULO-2742 offset history command by one
Posted by Christopher Tubbs <ct...@apache.org>.
> On April 28, 2014, 4:57 p.m., Christopher Tubbs wrote:
> > core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java, lines 76-81
> > <https://reviews.apache.org/r/20790/diff/2/?file=569586#file569586line76>
> >
> > This test fails.
I should have clarified. The assertion fails. The test does not throw an exception. The value of baos.toString().trim() appears to be the empty string.
- Christopher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20790/#review41635
-----------------------------------------------------------
On April 28, 2014, 3:40 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20790/
> -----------------------------------------------------------
>
> (Updated April 28, 2014, 3:40 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2742
> https://issues.apache.org/jira/browse/ACCUMULO-2742
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2742 offset history command by one
>
> The history entries returned by the history command are 0-indexed,
> while the history expansion is 1-indexed. We need to offset the index
> when we print it so that users can accurately use event expansion.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/util/shell/commands/HistoryCommand.java 9531d903aca834ab3b70650824023061e7e788d9
> core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20790/diff/
>
>
> Testing
> -------
>
> New unit test. Verified on cluster deployment.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 20790: ACCUMULO-2742 offset history command by one
Posted by Mike Drob <md...@mdrob.com>.
> On April 28, 2014, 8:57 p.m., Christopher Tubbs wrote:
> > core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java, lines 76-81
> > <https://reviews.apache.org/r/20790/diff/2/?file=569586#file569586line76>
> >
> > This test fails.
>
> Christopher Tubbs wrote:
> I should have clarified. The assertion fails. The test does not throw an exception. The value of baos.toString().trim() appears to be the empty string.
Hrm. Are you running from Eclipse or Maven command line? What Java? It works on my machine using both Eclipse and command line, and Oracle Java 6 and Java 7. Does it pass if you add a reader.flush() before the assertion?
- Mike
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20790/#review41635
-----------------------------------------------------------
On April 28, 2014, 7:40 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20790/
> -----------------------------------------------------------
>
> (Updated April 28, 2014, 7:40 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2742
> https://issues.apache.org/jira/browse/ACCUMULO-2742
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2742 offset history command by one
>
> The history entries returned by the history command are 0-indexed,
> while the history expansion is 1-indexed. We need to offset the index
> when we print it so that users can accurately use event expansion.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/util/shell/commands/HistoryCommand.java 9531d903aca834ab3b70650824023061e7e788d9
> core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20790/diff/
>
>
> Testing
> -------
>
> New unit test. Verified on cluster deployment.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 20790: ACCUMULO-2742 offset history command by one
Posted by Christopher Tubbs <ct...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20790/#review41635
-----------------------------------------------------------
core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java
<https://reviews.apache.org/r/20790/#comment75181>
This test fails.
- Christopher Tubbs
On April 28, 2014, 3:40 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20790/
> -----------------------------------------------------------
>
> (Updated April 28, 2014, 3:40 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2742
> https://issues.apache.org/jira/browse/ACCUMULO-2742
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2742 offset history command by one
>
> The history entries returned by the history command are 0-indexed,
> while the history expansion is 1-indexed. We need to offset the index
> when we print it so that users can accurately use event expansion.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/util/shell/commands/HistoryCommand.java 9531d903aca834ab3b70650824023061e7e788d9
> core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20790/diff/
>
>
> Testing
> -------
>
> New unit test. Verified on cluster deployment.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 20790: ACCUMULO-2742 offset history command by one
Posted by Christopher Tubbs <ct...@apache.org>.
> On April 28, 2014, 6:29 p.m., Christopher Tubbs wrote:
> > Ship It!
Actually... before you ship it, it'd be nice if there were a small comment explaining the assume usage in the event expansion test... just to explain why it doesn't work under some Eclipse environments.
- Christopher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20790/#review41644
-----------------------------------------------------------
On April 28, 2014, 6:11 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20790/
> -----------------------------------------------------------
>
> (Updated April 28, 2014, 6:11 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2742
> https://issues.apache.org/jira/browse/ACCUMULO-2742
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2742 offset history command by one
>
> The history entries returned by the history command are 0-indexed,
> while the history expansion is 1-indexed. We need to offset the index
> when we print it so that users can accurately use event expansion.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/util/shell/commands/HistoryCommand.java 9531d903aca834ab3b70650824023061e7e788d9
> core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20790/diff/
>
>
> Testing
> -------
>
> New unit test. Verified on cluster deployment.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 20790: ACCUMULO-2742 offset history command by one
Posted by Christopher Tubbs <ct...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20790/#review41644
-----------------------------------------------------------
Ship it!
Ship It!
- Christopher Tubbs
On April 28, 2014, 6:11 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20790/
> -----------------------------------------------------------
>
> (Updated April 28, 2014, 6:11 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2742
> https://issues.apache.org/jira/browse/ACCUMULO-2742
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2742 offset history command by one
>
> The history entries returned by the history command are 0-indexed,
> while the history expansion is 1-indexed. We need to offset the index
> when we print it so that users can accurately use event expansion.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/util/shell/commands/HistoryCommand.java 9531d903aca834ab3b70650824023061e7e788d9
> core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20790/diff/
>
>
> Testing
> -------
>
> New unit test. Verified on cluster deployment.
>
>
> Thanks,
>
> Mike Drob
>
>
Re: Review Request 20790: ACCUMULO-2742 offset history command by one
Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20790/
-----------------------------------------------------------
(Updated April 28, 2014, 10:11 p.m.)
Review request for accumulo.
Changes
-------
Verify that we are running in a supported terminal before using features.
Bugs: ACCUMULO-2742
https://issues.apache.org/jira/browse/ACCUMULO-2742
Repository: accumulo
Description
-------
ACCUMULO-2742 offset history command by one
The history entries returned by the history command are 0-indexed,
while the history expansion is 1-indexed. We need to offset the index
when we print it so that users can accurately use event expansion.
Diffs (updated)
-----
core/src/main/java/org/apache/accumulo/core/util/shell/commands/HistoryCommand.java 9531d903aca834ab3b70650824023061e7e788d9
core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/20790/diff/
Testing
-------
New unit test. Verified on cluster deployment.
Thanks,
Mike Drob
Re: Review Request 20790: ACCUMULO-2742 offset history command by one
Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20790/
-----------------------------------------------------------
(Updated April 28, 2014, 9:14 p.m.)
Review request for accumulo.
Changes
-------
Use platform independent new-line, because I think OS X behaves funny with JLine.
Bugs: ACCUMULO-2742
https://issues.apache.org/jira/browse/ACCUMULO-2742
Repository: accumulo
Description
-------
ACCUMULO-2742 offset history command by one
The history entries returned by the history command are 0-indexed,
while the history expansion is 1-indexed. We need to offset the index
when we print it so that users can accurately use event expansion.
Diffs (updated)
-----
core/src/main/java/org/apache/accumulo/core/util/shell/commands/HistoryCommand.java 9531d903aca834ab3b70650824023061e7e788d9
core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/20790/diff/
Testing
-------
New unit test. Verified on cluster deployment.
Thanks,
Mike Drob
Re: Review Request 20790: ACCUMULO-2742 offset history command by one
Posted by Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20790/
-----------------------------------------------------------
(Updated April 28, 2014, 7:40 p.m.)
Review request for accumulo.
Changes
-------
Added separate unit tests to check for separate conditions.
Bugs: ACCUMULO-2742
https://issues.apache.org/jira/browse/ACCUMULO-2742
Repository: accumulo
Description
-------
ACCUMULO-2742 offset history command by one
The history entries returned by the history command are 0-indexed,
while the history expansion is 1-indexed. We need to offset the index
when we print it so that users can accurately use event expansion.
Diffs (updated)
-----
core/src/main/java/org/apache/accumulo/core/util/shell/commands/HistoryCommand.java 9531d903aca834ab3b70650824023061e7e788d9
core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java PRE-CREATION
Diff: https://reviews.apache.org/r/20790/diff/
Testing
-------
New unit test. Verified on cluster deployment.
Thanks,
Mike Drob
Re: Review Request 20790: ACCUMULO-2742 offset history command by one
Posted by Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20790/#review41625
-----------------------------------------------------------
core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java
<https://reviews.apache.org/r/20790/#comment75157>
I'm confused - this appears to be testing that the ! event designator works, and not that the history is numbered correctly. (Sure, both should be tested.) I'd have figured there would be a test ensuring that the output was:
1: foo
2: bar
- Bill Havanki
On April 28, 2014, 3:18 p.m., Mike Drob wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20790/
> -----------------------------------------------------------
>
> (Updated April 28, 2014, 3:18 p.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2742
> https://issues.apache.org/jira/browse/ACCUMULO-2742
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2742 offset history command by one
>
> The history entries returned by the history command are 0-indexed,
> while the history expansion is 1-indexed. We need to offset the index
> when we print it so that users can accurately use event expansion.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/accumulo/core/util/shell/commands/HistoryCommand.java 9531d903aca834ab3b70650824023061e7e788d9
> core/src/test/java/org/apache/accumulo/core/util/shell/command/HistoryCommandTest.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/20790/diff/
>
>
> Testing
> -------
>
> New unit test. Verified on cluster deployment.
>
>
> Thanks,
>
> Mike Drob
>
>