add api for fetching federated query configs #339
Conversation
| import com.fasterxml.jackson.annotation.JsonCreator; | ||
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
| import com.fasterxml.jackson.databind.annotation.JsonDeserialize; | ||
| import com.google.common.base.Objects; |
There was a problem hiding this comment.
In td-client-java, we have abolished usage of com.google.common.base.Objects,
Could you rewrite the class without using com.google.common.base.Objects?
tiana528
force-pushed
the
federated-query-support
branch
from
ee048d8 to
38b0fef
Compare
src/main/java/com/treasuredata/client/deserialize/FederatedQueryConfigSettingsDeserializer.java
Outdated
Show resolved
Hide resolved
| @@ -1958,6 +1959,50 @@ public void testImportBytesWithId() | |||
| assertEquals(result.getUniqueId(), "4288048cf8f811e88b560a87157ac806"); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testGetFederatedQueryConfigsWhenEmpty() | |||
There was a problem hiding this comment.
Could you add test cases that the endpoint return 4xx status code?
The endpoint returns 401 and 403.
There was a problem hiding this comment.
It is not very convenient to simulate 401 and 403. I added a test case of 401, while it depends on the internal behaviour.
To support 401 and 403 in the test case, we need to configure the account of staging-aws used by the unit tests in a way to trigger 401 and 403 (Some junit tests in this class actually sends real request to staging-aws api3), which I feel might be a little too much.
| result = 31 * result + Objects.hashCode(createdAt); | ||
| result = 31 * result + Objects.hashCode(updatedAt); | ||
| result = 31 * result + Objects.hashCode(settings); | ||
| return result; |
There was a problem hiding this comment.
why not Objects.hash(id, type, userId, accountId, ...)
There was a problem hiding this comment.
In my understanding, that code is automatically generated by IntelliJ or from any template.
There was a problem hiding this comment.
Yes, it is generated by IntelliJ automatically.
There was a problem hiding this comment.
Changed to Objects.hash(id, type, userId, accountId, ...)
| private final String updatedAt; | ||
|
|
||
| @JsonDeserialize(using = com.treasuredata.client.deserialize.FederatedQueryConfigSettingsDeserializer.class) | ||
| private final String settings; |
There was a problem hiding this comment.
If this is anonymous json serialized, Map<String, Object> should be enough
|
Thank you! |
No description provided.