-
Notifications
You must be signed in to change notification settings - Fork 101
Fixing indices create partially #4652
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
base: main
Are you sure you want to change the base?
Conversation
Following you can find the validation changes for the APIs you have modified.
You can validate these APIs yourself by using the |
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.
Great job, thank you! In TokenFilterDefinition
, persian_stem
is missing. I think it's a newish stemmer.
* @aliases lang, language */ | ||
locale: string | ||
locale?: string |
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.
HunspellTokenFilter: locale has two aliases, and if it's set as mandatory as it was before, this means that the generator will generate 3 mandatory fields. For now I'm setting this as optional, but maybe we should support this as a feature?
Which generator? I think we should discuss this before making it wrongly optional
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.
ah sorry wrong wording, I mean the ts compiler. if you look at
export interface AnalysisHunspellTokenFilter extends AnalysisTokenFilterBase { |
export class RankVectorProperty extends PropertyBase { | ||
type: 'rank_vectors' | ||
element_type?: RankVectorElementType | ||
dims?: integer | ||
} | ||
|
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 is a technical preview feature, how can we specify it?
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.
could this work? since type is the internal tag
export class RankVectorProperty extends PropertyBase {
/**
* @availability stack since=8.18.0 stability=experimental
*/
type: 'rank_vectors'
element_type?: RankVectorElementType
dims?: integer
}
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.
Actually, I don't think that is possible today, see #2830. Leaving this for future work then.
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.
Can we add a comment at least?
@@ -534,44 +534,15 @@ export class SlowlogTresholdLevels { | |||
} | |||
|
|||
export class Storage { | |||
type: StorageType |
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.
The missing validated tests are either:
Details:
IcuCollationTokenFilter
's fields are actually snakeCase and not camel_case -> server codeHunspellTokenFilter
:locale
has two aliases, and if it's set as mandatory as it was before, this means that the generator will generate 3 mandatory fields. For now I'm setting this as optional, but maybe we should support this as a feature?PatternReplaceTokenFilter
was missing theflags
field -> server codeEdgeNGramTokenizer
, token_chars can also be expressed as a csv, so making it astring | enum
unionRankVectorProperty
was missing -> server codeFlattenedProperty
was missingtime_series_dimensions
-> server codeAggregateMetricDoubleProperty
was missingignore_malformed
-> server codeGeoPointProperty
was missingtime_series_metric
-> server codeStorage
inIndexSettings
was missingstats_refresh_interval
, but I also noticed that in the code and yaml tests there was no trace of thetype
field, nor theStorageType
enum, so maybe we can remove that. -> server code