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

remove create report logic for background job #193

Merged

Conversation

zhongnansu
Copy link
Member

@zhongnansu zhongnansu commented Nov 13, 2020

Issue #, if available:
Since we don't have user impersonation now, and we only send a link instead of actual file by email, I remove the logic which causing error when security is enabled

Description of changes:

  • remove create report logic for background job
  • leave only delivery report logic
  • refine few logging and error handling

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

@zhongnansu zhongnansu marked this pull request as ready for review November 13, 2020 00:46
);
}
// TODO: generate report logic will be added back once we have the user impersonation
// if (reportSource === REPORT_TYPE.savedSearch) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to leave this commented-out code in?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I leave it on purpose, once we have user impersonation added, this code snippet will be reused.

Copy link
Contributor

@davidcui1225 davidcui1225 left a comment

Choose a reason for hiding this comment

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

LGTM- just one minor comment

@zhongnansu zhongnansu merged commit 1e6ba4b into opendistro-for-elasticsearch:dev Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants