-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Java client issues batch 2 #2440
Conversation
3d47da4
to
3cc0fe4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure about GeoPointProperty
, it seems to fit the server description, maybe these should be a level higher.
@@ -18,5 +18,5 @@ | |||
*/ | |||
|
|||
export class TransientMetadataConfig { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a Dictionary<string, UserDefinedValue>
https://github.com/elastic/elasticsearch/blob/46beceb18024a27a57449de6e45897a0fb1d890b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authz/RoleDescriptor.java#L68
|
||
export class Role { | ||
cluster: string[] | ||
indices: IndicesPrivileges[] | ||
metadata: Metadata | ||
run_as: string[] | ||
transient_metadata: TransientMetadataConfig | ||
transient_metadata?: Dictionary<string, UserDefinedValue> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Anaethelion I also applied the same change to the security spec since the class should be the same, is it correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Also remove TransientMetadataConfig
that is now useless.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
Following you can find the validation results for the APIs you have changed.
You can validate these APIs yourself by using the |
* making status in enrich policy response optional (issue 744, 376) * making transient_metadata and enabled field in role response optional (issue 317) * byte fields from integer to long (issue 379) * adding missing properties to GeoPoint (issue 394) * making source in InlineGet optional (issue 403) * stored_field should be stored_fields in InnerHits (issue 409) * added positive_score_impact to RankFeaturesProperty (issue 450) * making dims optional for DenseVectorProperty (issue 754) * TransientMetadataConfig as map * removed duplicate schema json (cherry picked from commit 0a63a19)
* making status in enrich policy response optional (issue 744, 376) * making transient_metadata and enabled field in role response optional (issue 317) * byte fields from integer to long (issue 379) * adding missing properties to GeoPoint (issue 394) * making source in InlineGet optional (issue 403) * stored_field should be stored_fields in InnerHits (issue 409) * added positive_score_impact to RankFeaturesProperty (issue 450) * making dims optional for DenseVectorProperty (issue 754) * TransientMetadataConfig as map * removed duplicate schema json
Fixes provided in this PR:
status
optional, otherwise when called withwaitForCompletion=false
the parsing will failtransient_metadata
should be a map, not a single field class. Starting a basic elasticsearch instance withxpack.security.enabled
and a password and trying to callclient.security().getRole()
will otherwise fail.*_size_bytes
fields should belong
, notinteger
source
in InlineGet should be optionalstored_field
in InnerHits should bestored_fields
positive_score_impact
like RankFeatureProperty doesdims
should be optional for dense vectors