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.

Sign up for GitHub

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

Jump to bottom

[MDS] Add support for register data sources during the absence of local cluster #2140

Merged
merged 4 commits into from
Sep 6, 2024
Merged

[MDS] Add support for register data sources during the absence of local cluster #2140

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
395190a
Add support for rigister plugin during the absence of local cluster
RyanL1997 Sep 6, 2024
2047b36
Add release note update
RyanL1997 Sep 6, 2024
08174f4
remove console log
RyanL1997 Sep 6, 2024
b61ca6f
fix link checker
RyanL1997 Sep 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 50 additions & 45 deletions public/plugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ export class ObservabilityPlugin
constructor(initializerContext: PluginInitializerContext) {
this.config = initializerContext.config.get<PublicConfig>();
}
private featureFlagStatus: boolean = false;
Copy link
Member

Choose a reason for hiding this comment

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

can this variable naming be more explicit? looking at featureFlagStatus it's hard to tell what the features is..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I will change that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since the backport is failing, I will manually change it in 2.x first and backport. I will push another commit for change it on main.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have updated here #2144.


public setup(
core: CoreSetup<AppPluginStartDependencies>,
Expand All @@ -184,6 +185,8 @@ export class ObservabilityPlugin
});

setupOverviewPage(setupDeps.contentManagement!);
this.featureFlagStatus = !!setupDeps.dataSource;
console.log('Feature Flag Status:', this.featureFlagStatus);
RyanL1997 marked this conversation as resolved.
Show resolved Hide resolved

// redirect legacy notebooks URL to current URL under observability
if (window.location.pathname.includes('notebooks-dashboards')) {
Expand Down Expand Up @@ -459,55 +462,57 @@ export class ObservabilityPlugin
return `${type.charAt(0).toUpperCase()}${type.slice(1)}`;
};

// register all s3 datasources
const registerDataSources = () => {
try {
core.http.get(`${DATACONNECTIONS_BASE}`).then((s3DataSources) => {
s3DataSources.map((s3ds) => {
dataSourceService.registerDataSource(
dataSourceFactory.getDataSourceInstance(S3_DATA_SOURCE_TYPE, {
id: htmlIdGenerator(OBS_S3_DATA_SOURCE)(),
name: s3ds.name,
type: s3ds.connector.toLowerCase(),
metadata: {
...s3ds.properties,
ui: {
label: s3ds.name,
typeLabel: getDataSourceTypeLabel(s3ds.connector.toLowerCase()),
groupType: s3ds.connector.toLowerCase(),
selector: {
displayDatasetsAsSource: false,
// register all s3 datasources only if mds feature flag is disabled
if (!this.featureFlagStatus) {
Copy link
Member

Choose a reason for hiding this comment

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

can we directly use setupDeps.dataSource here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we can't access the setupDeps from the start function, we can only access to the startDeps, However, the startDeps.dataSource.dataSourceEnabled is also not accessible, when the dataSource plugin is not registered.

Copy link
Member

Choose a reason for hiding this comment

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

got it. this makes sense for now. Ideally with should stop the registration at setup itself. Adding if condition here: https://github.com/opensearch-project/dashboards-observability/blob/3ad2bf0448f6a705f449f42e62be8398f840ea37/public/plugin.tsx#L264C11-L264C31

const registerDataSources = () => {
try {
core.http.get(`${DATACONNECTIONS_BASE}`).then((s3DataSources) => {
s3DataSources.map((s3ds) => {
dataSourceService.registerDataSource(
dataSourceFactory.getDataSourceInstance(S3_DATA_SOURCE_TYPE, {
id: htmlIdGenerator(OBS_S3_DATA_SOURCE)(),
name: s3ds.name,
type: s3ds.connector.toLowerCase(),
metadata: {
...s3ds.properties,
ui: {
label: s3ds.name,
typeLabel: getDataSourceTypeLabel(s3ds.connector.toLowerCase()),
groupType: s3ds.connector.toLowerCase(),
selector: {
displayDatasetsAsSource: false,
},
},
},
},
})
);
})
);
});
});
});
} catch (error) {
console.error('Error registering S3 datasources', error);
}
};

dataSourceService.registerDataSourceFetchers([
{ type: S3_DATA_SOURCE_TYPE, registerDataSources },
]);

if (startDeps.securityDashboards) {
core.http
.get(SECURITY_PLUGIN_ACCOUNT_API)
.then(() => {
registerDataSources();
})
.catch((e) => {
if (e?.response?.status !== 401) {
// accounts api should not return any error status other than 401 if security installed,
// this datasource register is included just in case
} catch (error) {
console.error('Error registering S3 datasources', error);
}
};

dataSourceService.registerDataSourceFetchers([
{ type: S3_DATA_SOURCE_TYPE, registerDataSources },
]);

if (startDeps.securityDashboards) {
core.http
.get(SECURITY_PLUGIN_ACCOUNT_API)
.then(() => {
registerDataSources();
}
});
} else {
registerDataSources();
})
.catch((e) => {
if (e?.response?.status !== 401) {
// accounts api should not return any error status other than 401 if security installed,
// this datasource register is included just in case
registerDataSources();
}
});
} else {
registerDataSources();
}
}

core.http.intercept({
Expand Down
Loading