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

[MDS] Fix showing DQS sources list in workspaces #8838

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

Conversation

jowg-amazon
Copy link
Contributor

@jowg-amazon jowg-amazon commented Nov 9, 2024

Description

  • Fixes showing the list of dqs sources in workspaces
Screenshot 2024-11-09 at 2 10 59 PM

Issues Resolved

Screenshot

Testing the changes

Changelog

  • fix: [MDS] Fix showing DQS sources list in workspaces

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Copy link
Contributor

github-actions bot commented Nov 9, 2024

ℹ️ Manual Changeset Creation Reminder

Please ensure manual commit for changeset file 8838.yml under folder changelogs/fragments to complete this PR.

If you want to use the available OpenSearch Changeset Bot App to avoid manual creation of changeset file you can install it in your forked repository following this link.

For more information about formatting of changeset files, please visit OpenSearch Auto Changeset and Release Notes Tool.

Signed-off-by: jowg-amazon <[email protected]>
Copy link

codecov bot commented Nov 9, 2024

Codecov Report

Attention: Patch coverage is 64.00000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 60.94%. Comparing base (42df421) to head (49cd923).

Files with missing lines Patch % Lines
...ion/manage_direct_query_data_connections_table.tsx 65.21% 4 Missing and 4 partials ⚠️
.../data_source_management/public/components/utils.ts 50.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8838   +/-   ##
=======================================
  Coverage   60.93%   60.94%           
=======================================
  Files        3800     3800           
  Lines       90878    90892   +14     
  Branches    14324    14326    +2     
=======================================
+ Hits        55377    55390   +13     
- Misses      31971    31972    +1     
  Partials     3530     3530           
Flag Coverage Δ
Linux_1 29.01% <0.00%> (-0.01%) ⬇️
Linux_2 56.39% <ø> (ø)
Linux_3 37.93% <64.00%> (+<0.01%) ⬆️
Linux_4 29.00% <0.00%> (-0.01%) ⬇️
Windows_1 29.02% <0.00%> (-0.01%) ⬇️
Windows_2 56.34% <ø> (ø)
Windows_3 37.93% <64.00%> (+<0.01%) ⬆️
Windows_4 29.00% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Nov 9, 2024

❌ Empty Changelog Section

The Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section.

Copy link
Contributor

github-actions bot commented Nov 9, 2024

❌ Changelog Entry Missing Hyphen

Changelog entries must begin with a hyphen (-).

Copy link
Contributor

github-actions bot commented Nov 9, 2024

❌ Entry Too Long

Entry is 130 characters long, which is 30 characters longer than the maximum allowed length of 100 characters. Please revise your entry to be within the maximum length.

Comment on lines +152 to +154
{
S3GLUE: 'Amazon S3',
PROMETHEUS: 'Prometheus',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Plz use DatasourceTypeToDisplayName

Comment on lines +175 to +186
const fetchDirectQueryConnections = async (): Promise<DataSourceTableItem[]> => {
try {
const dataConnectionSavedObjects = await getDataConnections(savedObjects.client);
return dataConnectionSavedObjects.map((obj) => ({
id: obj.id,
title: obj.attributes.connectionId,
type: obj.attributes.type,
}));
} catch (error: any) {
return [];
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should make sure this is covered as its new functionality

Comment on lines +408 to +414
if (
record.type === DataConnectionType.SecurityLake ||
record.type === DataConnectionType.CloudWatch
) {
// TODO: link to details page for security lake and cloudwatch
return <span style={indentStyle}> {name}</span>;
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should make sure this is covered as its new functionality

@@ -181,6 +182,17 @@ export async function getDataSources(savedObjectsClient: SavedObjectsClientContr
);
}

export async function getDataConnections(savedObjectsClient: SavedObjectsClientContract) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we type response?

Comment on lines +186 to +193
return savedObjectsClient
.find<DataConnectionSavedObjectAttributes>({
type: 'data-connection',
perPage: 10000,
})
.then((response) => {
return response?.savedObjects;
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

given this is new, can we cover with tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants