Generate more indices operations #1358
Conversation
Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Xtansia
requested review from
reta,
Bukhtawar,
dblock,
szczepanczykd,
madhusudhankonda,
saratvemulapalli and
VachaShah
as code owners
java-client/src/generated/java/org/opensearch/client/opensearch/indices/AddBlockRequest.java
Show resolved
Hide resolved
| op.add(Builder::analyzer, JsonpDeserializer.stringDeserializer(), "analyzer"); | ||
| op.add(Builder::attributes, JsonpDeserializer.arrayDeserializer(JsonpDeserializer.stringDeserializer()), "attributes"); | ||
| op.add(Builder::charFilter, JsonpDeserializer.arrayDeserializer(CharFilter._DESERIALIZER), "char_filter"); | ||
| op.add(Builder::explain, JsonpDeserializer.booleanDeserializer(), "explain"); | ||
| op.add(Builder::field, JsonpDeserializer.stringDeserializer(), "field"); | ||
| op.add(Builder::filter, JsonpDeserializer.arrayDeserializer(TokenFilter._DESERIALIZER), "filter"); | ||
| op.add(Builder::index, JsonpDeserializer.stringDeserializer(), "index"); |
There was a problem hiding this comment.
The index is not part of the JSON payload, is it? (it is URL path parameter)
There was a problem hiding this comment.
Correct, we had a few request classes that'd been hand edited to allow deserializing the path params from JSON, so had incorporated it without to avoid breaking changes. See #723
The problem we run into is that requests which have required path parameters are impossible to deserialize otherwise due to the required fields.
But can revert it everywhere if you think?
There was a problem hiding this comment.
The index is not part of the JSON payload, is it? (it is URL path parameter)
I think the problem here is that this is not the JSON that will be sent to the server but a workaround to restore the state of the object. Yes, I would revert it everywhere, the path / query / etc params should not be part of the request payload (unless they are repeated for some reasons). Thank you.
There was a problem hiding this comment.
They're only deserializable, they won't be serialized into the JSON sent to the server
There was a problem hiding this comment.
Exactly, so when they will be serialized into the JSON? More specifically, under which conditions such properties (like index) are going to be serialized (and later deserialized)?
There was a problem hiding this comment.
I believe that it's being used to construct the request shapes from externally generated JSON.
If we're going to remove the ability to deserialize the path parameters, then we should completely remove deserialization from the request objects as it's unusable in the majority of cases otherwise. Or provide some other mechanism to pass the path (& query) params separate to the JSON payload for deserialization
There was a problem hiding this comment.
Or provide some other mechanism to pass the path (& query) params separate to the JSON payload for deserialization
Yes, I think that would be the right option.
| private final List<ExpandWildcard> expandWildcards; | ||
|
|
||
| @Nullable | ||
| private final Boolean ignoreUnavailable; | ||
|
|
||
| @Nonnull | ||
| private final List<String> index; |
There was a problem hiding this comment.
This seems to be also not in sync? Shouldn't be indices? https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/action/admin/indices/alias/get/GetAliasesRequest.java#L53
| @Nullable | ||
| private final Query filter; | ||
|
|
||
| @Nonnull | ||
| private final List<String> index; |
There was a problem hiding this comment.
Signed-off-by: Thomas Farr <[email protected]>
| import org.junit.Test; | ||
| import org.opensearch.client.json.JsonpMapper; | ||
| import org.opensearch.client.json.jsonb.JsonbJsonpMapper; | ||
| import org.opensearch.client.opensearch.indices.PutIndexTemplateRequest; | ||
|
|
||
| public class PutIndexTemplateRequestTest extends Assert { | ||
|
|
||
| // TODO: Allow constructing requests from JSON with required path params | ||
| @Ignore |
There was a problem hiding this comment.
I think we could check that path params are not deserialized instead of ignoring
Signed-off-by: Thomas Farr <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Xtansia
force-pushed
the
feat/code-gen/indices
branch
from
628b246 to
c53f5a4
Compare
Description
Generate more indices operations
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.