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

Discover Changes for Point in Time POC #4

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Arpit-Bandejiya
Copy link
Owner

@Arpit-Bandejiya Arpit-Bandejiya commented Oct 20, 2022

Signed-off-by: Arpit Bandejiya abandeji@amazon.com

Description

This PR deal with the integration of Point in Time plugin feature in the discover page. These code changes are not independent and will require the rest of the Plugin to work with. This PR primarily for review purposes for the design of discover page.

Meta issue

Signed-off-by: Arpit Bandejiya <abandeji@amazon.com>
export function getSwitchIndexPatternAppState(
currentIndexPattern: IndexPattern,
nextIndexPattern: IndexPattern,
currentIndexPattern: IndexPattern | PointInTime,
Copy link
Owner Author

Choose a reason for hiding this comment

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

In the main implementation the aim is to have the discover page react to IndexPattern and Point in Time similarly. So Here we are thinking to abstract IndexPattern and Point in Time object and have a parent interface which can be used.

Choose a reason for hiding this comment

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

Need a separate field for point in time instead of reusing "indexpattern" fields

const nextColumns = modifyColumns
? currentColumns.filter(
(column) =>
if (nextIndexPattern.attributes) {
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is a crude way to check whether it's a index pattern or point in time. Will have to do a better implementation for the same

$scope.indexPattern = resolveIndexPatternLoading();
$scope.selectedPointInTime = $route.current.locals.savedObjects.pit.pitid || undefined;
Copy link
Owner Author

Choose a reason for hiding this comment

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

If there is no pitid in the state, make it undefined

@@ -294,20 +330,49 @@ function discoverController($element, $route, $scope, $timeout, $window, Promise
});

$scope.setIndexPattern = async (id) => {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Ignore this function since the same legacy code.

@@ -102,6 +103,33 @@ const fetchStatuses = {

const app = getAngularModule();

export async function getpits(savedObjectsClient) {

This comment was marked as resolved.

@@ -77,6 +77,7 @@ const {
toastNotifications,
uiSettings: config,
visualizations,
savedObjectsClient,
Copy link
Owner Author

Choose a reason for hiding this comment

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

We will need savedObjectClient to fetch the PIT saved object. Since we already have the necessary dependencies in the plugin. We have imported it in the discover page

const params = toSnakeCase({
...defaultParams,
...getShardTimeout(config),
...request.params,
});

if (params.body.pit) {
Copy link
Owner Author

@Arpit-Bandejiya Arpit-Bandejiya Oct 20, 2022

Choose a reason for hiding this comment

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

We will need to make changes in the search strategy as well since there are some fields which can't be passed in PIT search.

@@ -49,7 +49,7 @@ export async function getDefaultSearchParams(uiSettingsClient: IUiSettingsClient
maxConcurrentShardRequests:
maxConcurrentShardRequests > 0 ? maxConcurrentShardRequests : undefined,
ignoreThrottled,
ignoreUnavailable: true, // Don't fail if the index/indices don't exist
ignoreUnavailable: false, // Don't fail if the index/indices don't exist
Copy link
Owner Author

Choose a reason for hiding this comment

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

Need to make a different function to be called for fetching the default params for PIT searches.

Signed-off-by: Arpit Bandejiya <abandeji@amazon.com>
Arpit-Bandejiya pushed a commit that referenced this pull request May 19, 2023
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