Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Get All and Explain All #149

Merged
merged 13 commits into from
Feb 11, 2021
Merged

Get All and Explain All #149

merged 13 commits into from
Feb 11, 2021

Conversation

bowenlan-amzn
Copy link
Contributor

@bowenlan-amzn bowenlan-amzn commented Dec 17, 2020

Issue #, if available:

Description of changes:

Following methods in the kibana server Services needs to be changed to use our backend API only:

  • ManagedIndexService: getManagedIndices now use backend explainAll, and use getPolicy to fill in policy field (we don't have policy saved in explain metadata)
    • edge case, when index managed by a non-existing policy, this getPolicy call will throw exception, I catch it and only log and info saying policy not found for this managed index
  • PolicyService: getPolicies now use backend getPolicies
  • IndexService:
    searchPolicies(frontend) now use getPolicies
    _getManagedStatus now use backend explainAll; getIndices use _getManagedStatus

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@bowenlan-amzn bowenlan-amzn added the enhancement New feature or request label Jan 27, 2021
throw err;
}
}
policy = _.get(getResponse, "policy", null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Unclear what the behavior here is on the UI when the explain returns a response and the get fails and we have policy: null. I think before a policy could be null when it hadn't initialized yet, is that still possible? Can it be confusing now since policy: null on the UI can mean not initialized vs failed to get.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously policy is from managed-index doc, now it's directly get from policy doc. So now, if policy is null, will always show "Failed to load policy", it actually means policy not exist. If get policy call fail, it will throw an exception.

For initializing, check if metadata contain policy seqNo, if it's null, it means we are still initializing

Base automatically changed from master to main February 8, 2021 16:45
// If the config index does not exist then nothing is being managed
if (err.statusCode === 404 && err.body.error.type === "index_not_found_exception") {
return indexUuids.reduce((accu, value) => ({ ...accu, [value]: "No" }), {});
const explainParamas = { index: indexNames.toString() };
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a typo? I am assuming that this should be explainParams

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, will fix it in today's bump version small PR

return indexUuids.reduce((accu, value) => ({ ...accu, [value]: "No" }), {});
const explainParamas = { index: indexNames.toString() };
const { callAsCurrentUser: callWithRequest } = this.esDriver.asScoped(request);
const explainResponse: ExplainResponse = await callWithRequest("ism.explain", explainParamas);
Copy link
Contributor

Choose a reason for hiding this comment

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

Also here explaingParamas -> explainParams

@bowenlan-amzn bowenlan-amzn merged commit 46e3c2a into opendistro-for-elasticsearch:main Feb 11, 2021
@bowenlan-amzn bowenlan-amzn deleted the getall branch February 11, 2021 17:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants