Details
Major Description
We (Java SDK folk) are now running our CI tests against 6.6, which has found this minor regression.
The test is:
@Test
|
void dropDatasetFailsIfAbsent() { |
assertThrows(DatasetNotFoundException.class, () -> analytics.dropDataset("foo", |
dropDatasetAnalyticsOptions()
|
.dataverseName("absentDataverse"))); |
}
|
|
In 6.6 this now fails as it raises a DataverseNotFoundException rather than DatasetNotFoundException, as cbas now returns a 24034 code. In this test and the one below too, neither "foo" nor "absentDataverse" exist.
This causes problems for our `ignoreIfNotExist` option, which allows dropping datasets and silently ignoring if they don't exist. Because that logic is expecting a DataverseNotFoundException.
E.g. this other test fails with a DataverseNotFoundException, when you would expect it to succeed. This test also passed in 6.5.
@Test
|
void dropDatasetCanIgnoreAbsent() { |
analytics.dropDataset("foo", |
dropDatasetAnalyticsOptions()
|
.ignoreIfNotExists(true) |
.dataverseName("absentDataverse")); |
}
|
|
For both tests, if I don't specify a dataverseName then the tests pass as expected.
Attachments
Activity
| Field | Original Value | New Value |
|---|---|---|
| Priority | Minor [ 4 ] | Major [ 3 ] |
| Description |
We (Java SDK folk) are now running our CI tests against 6.6, which has found this minor regression.
The test is: {code:java} @Test void dropDatasetFailsIfAbsent() { assertThrows(DatasetNotFoundException.class, () -> analytics.dropDataset("foo", dropDatasetAnalyticsOptions() .dataverseName("absentDataverse"))); } {code} In 6.6 this now fails as it raises a DataverseNotFoundException rather than DatasetNotFoundException, as cbas now returns a 24034 code. This causes problems for our `ignoreIfNotExist` option, which allows dropping datasets and silently ignoring if they don't exist. Because that logic is expecting a DataverseNotFoundException. E.g. this other test fails with a DataverseNotFoundException, when you would expect it to succeed (neither "foo" nor "absentDataverse" exist). This test also passed in 6.5. {code:java} @Test void dropDatasetCanIgnoreAbsent() { analytics.dropDataset("foo", dropDatasetAnalyticsOptions() .ignoreIfNotExists(true) .dataverseName("absentDataverse")); } {code} |
We (Java SDK folk) are now running our CI tests against 6.6, which has found this minor regression.
The test is: {code:java}@Test void dropDatasetFailsIfAbsent() { assertThrows(DatasetNotFoundException.class, () -> analytics.dropDataset("foo", dropDatasetAnalyticsOptions() .dataverseName("absentDataverse"))); } {code} In 6.6 this now fails as it raises a DataverseNotFoundException rather than DatasetNotFoundException, as cbas now returns a 24034 code. In this test and the one below too, neither "foo" nor "absentDataverse" exist. This causes problems for our `ignoreIfNotExist` option, which allows dropping datasets and silently ignoring if they don't exist. Because that logic is expecting a DataverseNotFoundException. E.g. this other test fails with a DataverseNotFoundException, when you would expect it to succeed. This test also passed in 6.5. {code:java}@Test void dropDatasetCanIgnoreAbsent() { analytics.dropDataset("foo", dropDatasetAnalyticsOptions() .ignoreIfNotExists(true) .dataverseName("absentDataverse")); } {code} For both tests, if I don't specify a dataverseName then the tests pass as expected. |
| Rank | Ranked higher |
| Affects Version/s | 6.6.0 [ 16787 ] |
| Labels | triaged |
| Assignee | Till Westmann [ till ] | Dmitry Lychagin [ dmitry.lychagin ] |
Dmitry, thanks for the reply. I maybe agree with you on the specificity being better, but it is awkward for us to fix our side due to the changes needing to go into multiple SDKs, plus we need to update our sdk-rfcs detailing the protocol. I will raise with the team and get back to you.
Actually, I need to revisit the problem description. I think there are two separate problems here. Problem 1 is that this code:
analytics.dropDataset("absentDataset", dropDatasetAnalyticsOptions().dataverseName("absentDataverse")) |
used to raise DatasetNotFoundException and now raises DataverseNotFoundException.
I saw the ignoreIfNotExists test was failing also and assumed it was caused by the same change. I'm rusty on the analytics manager code and thought it worked the same as N1QL, where we implement ignoreIfNotExists by ignoring a specific error code. But analytics is different, it supports "IF EXISTS" syntax here, which we use.
So in fact I think there is a separate problem/regression 2 here, where
analytics.dropDataset("absentDataset", dropDatasetAnalyticsOptions().ignoreIfNotExists().dataverseName("absentDataverse")) |
which results in a single statement with "IF EXISTS" being sent to cbas, used to return success but now returns an error code causing us to raise DataverseNotFoundException.
If all that makes sense?
So to clarify, the ask from analytics team is that all SDKs:
1. For problem 1: I'm not sure? Make no change and just accept that this is a minor breaking change for users in this edge case?
2. For problem 2: ignore the 24034 (dataverse not found) error code if doing a "DROP DATASET ... IF EXISTS" statement?
| Fix Version/s | 6.6.0 [ 16787 ] |
| Is this a Regression? | Unknown [ 10452 ] | Yes [ 10450 ] |
| Status | Open [ 1 ] | In Progress [ 3 ] |
The change in IF EXISTS behavior (problem 2) is a regression. We're working on fixing this in 6.6.
As for problem 1, our proposal is to leave it as is. So if user does not request "ignoreIfNotExists" and the dataverse does not exist, then the user will get DataverseNotFoundException instead of previous DatasetNotFoundException.
| Link | This issue blocks MB-38724 [ MB-38724 ] |
| Labels | triaged | approved-for-6.6.0 triaged |
| Due Date | 24/Jul/20 |
| Sprint | CX Sprint 209 [ 1167 ] |
| Link | This issue relates to MB-40592 [ MB-40592 ] |
Ok Dmitry Lychagin, I'll let the team know and let you know if they have any feedback. That sounds ok to me though, I'll modify these tests accordingly. MB-40576 is more of a problem as we'll have to find some way of disabling the analytics tests, or a subset of them, but only on 6.6.0.
| Resolution | Fixed [ 1 ] | |
| Status | In Progress [ 3 ] | Resolved [ 5 ] |
| Assignee | Dmitry Lychagin [ dmitry.lychagin ] | Arunkumar Senthilnathan [ arunkumar ] |
| Labels | approved-for-6.6.0 triaged | approved-for-6.6.0 combination_testing triaged |
| Assignee | Arunkumar Senthilnathan [ arunkumar ] | Will Broadbelt [ will.broadbelt ] |
Verified tests pass with 6.6.0-7897.
| Status | Resolved [ 5 ] | Closed [ 6 ] |
This behavior was changed by https://issues.couchbase.com/browse/MB-39142 , specifically:
https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/6203/8/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/QueryTranslator.java#1576
Graham Pople,
We discussed this issue today and believe the new error code better explains the problem to the end user because it's more specific ("dataverse not found" instead of "dataset not found").
Could you consider fixing it on your side? If ignoreIfNotExists=true then do not fail on either DataverseNotFoundException or DatasetNotFoundException?