Ready: Update TS objects and Commands for TS 1.4 #651
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alexmoore
changed the title
alexmoore
assigned AleksandrPavlenko, christophermancini, lukebakken and srgg and unassigned AleksandrPavlenko
| @@ -80,13 +81,18 @@ private static ColumnDescription convertPBColumnDescription(RiakTsPB.TsColumnDes | |||
| private static FullColumnDescription convertDescribeResultRowToFullColumnDescription(Row row) | |||
| { | |||
| /* | |||
| * Expected Format for the DESCRIBE function is 5 columns: | |||
| * Expected Format for the DESCRIBE function is 5 or 7 columns depending on the version. | |||
There was a problem hiding this comment.
IMO it worth to mention Riak TS version that support each format
There was a problem hiding this comment.
Isn't this eventually going to be 8 cols in 1.4?
There was a problem hiding this comment.
The ASC/DESC features got moved to 1.5.
2.1+", was meant for a new branch. This reverts commit d02264d.
| @@ -338,7 +337,7 @@ public void test_p_TestDescribeTable() throws InterruptedException, ExecutionExc | |||
|
|
|||
| final QueryResult tableDescription = resultFuture.get(); | |||
| assertEquals(7, tableDescription.getRowsCount()); | |||
| assertEquals(5, tableDescription.getColumnDescriptionsCopy().size()); | |||
| assertEquals(7, tableDescription.getColumnDescriptionsCopy().size()); | |||
There was a problem hiding this comment.
Throws failure on TS 1.3.1, actual = 5.
There was a problem hiding this comment.
@christophermancini Ok, try the new commit.
|
👍 |
|
Still getting errors against 1.3.1 with the latest commit. |
|
Tests against Riak KV 2.1.4 pass. |
|
Hmm, tests locally against TS 1.3.1 & TS 1.4 work. |
|
The Siblings issue could be caused by a slow Riak too, we use ListKeys + MultiDelete to clean up, so if something isn't listed in time for deletion you could get siblings when the next test run's data is written. |
|
Going to merge for now. |
|
@christophermancini Almost forgot, but I was getting some failures on 1.3.1, when I ran a single node - the coverage api tests won't work with that test configuration. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add Local Key ASC/DESC ordering to the DescribeTable command's results.Moved to 1.5