Skip to content
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

Job for reindexing elastic indexes & move index aliases #258

Merged
merged 9 commits into from
Feb 18, 2019

Conversation

Tobsucht
Copy link
Contributor

No description provided.

@Tobsucht Tobsucht added the 🖐 Keep open Should not be merged label Feb 15, 2019
@Tobsucht Tobsucht removed the 🖐 Keep open Should not be merged label Feb 15, 2019
Copy link
Member

@andyHa andyHa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...

@Override
protected void collectParameters(Consumer<Parameter<?, ?>> parameterCollector) {
if (stringParameter == null) {
stringParameter = new StringParameter("destination", NLS.get("MoveIndexAliasJobFactory.destinationParameter"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not cache the parameter here, as NLS.get is dependent on the user locale - so the first user would determine the parameter name for all successive calls

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simply use collector.accept(new StringParameter(...).markRequired())

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or even better - initialize the fields directly and uses a constant for label as it will be auto translated

@Override
protected void collectParameters(Consumer<Parameter<?, ?>> parameterCollector) {
if (entityDescriptorParameter == null) {
entityDescriptorParameter = new ElasticEntityDescriptorParameter("ed", NLS.get("ReindexJobFactory.descriptorParameter"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s.a.

private Elastic elastic;

private ElasticEntityDescriptorParameter entityDescriptorParameter = null;
private StringParameter stringParameter = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private StringParameter stringParameter = null;
private StringParameter stringParameter = new StringParameter("destination", "MoveIndexAliasJobFactory.destinationParameter");

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • .markRequired()

@Part
private Elastic elastic;

private ElasticEntityDescriptorParameter entityDescriptorParameter = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private ElasticEntityDescriptorParameter entityDescriptorParameter = null;
private ElasticEntityDescriptorParameter entityDescriptorParameter = new ElasticEntityDescriptorParameter("ed", "MoveIndexAliasJobFactory.descriptorParameter").markRequired();

* Implements a job which reindexes a given index in elastic.
*/
@Register(classes = JobFactory.class)
public class ReindexJobFactory extends BatchProcessJobFactory {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Permission(TenantUserManager.PERMISSION_SYSTEM_TENANT)


@Override
public String getCategory() {
return "MAINTENANCE";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JobCategory.CATEGORY_MISC

- Use defined JobCategory
- Don't cache job parameters as translations could be wrong
- Add permission checks
Copy link
Member

@andyHa andyHa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...

(ElasticEntityDescriptorParameter) new ElasticEntityDescriptorParameter("ed",
"$MoveIndexAliasJobFactory.descriptorParameter")
.markRequired();
private StringParameter stringParameter =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

destinationParameter


@Override
protected String createProcessTitle(Map<String, String> context) {
return "MoveIndexAlias";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something like Strings.apply("Movin Elasticsearch Index %s to %s, ....) - would be very nice


@Override
protected String createProcessTitle(Map<String, String> context) {
return "Reindex";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s.a.

@andyHa andyHa merged commit f7a512f into master Feb 18, 2019
@andyHa andyHa deleted the tba/reindex-aliasing branch February 18, 2019 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants