Add resource API pattern #9731

kathyguo18 · 2025-04-25T23:00:55Z

Description

feat. Add resource API pattern in query_enhancements

Issues Resolved

Screenshot

Testing the changes

yarn test

Changelog

Add resource API pattern in query_enhancements

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

github-actions · 2025-04-25T23:01:30Z

ℹ️ Manual Changeset Creation Reminder

Please ensure manual commit for changeset file 9731.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.

src/plugins/query_enhancements/server/routes/resources/routes.ts

joshuali925 · 2025-04-25T23:06:20Z

src/plugins/query_enhancements/server/plugin.ts

+      registerResourceManager: (dataConnectionType: string, manager: BaseConnectionManager) =>
+        resourceManagerService.register(dataConnectionType, manager),


this seems to be

Suggested change

registerResourceManager: (dataConnectionType: string, manager: BaseConnectionManager) =>

resourceManagerService.register(dataConnectionType, manager),

registerResourceManager: resourceManagerService.register,

I'm not sure about the reason. But when using resourceManagerService.register directly, it goes to the method itself instead of the resourceManagerService singleton.

The error is like:

TypeError: Cannot read properties of undefined (reading 'type') at Object.registerResourceManager at AwsapmdqsdashboardsPluginPlugin.setup

src/plugins/query_enhancements/server/routes/resources/routes.ts

joshuali925 · 2025-04-25T23:10:44Z

src/plugins/query_enhancements/server/connections/resource_manager_service.ts

+    if (this.managers[dataConnectionType] !== undefined) {
+      throw new Error(
+        `Manager for dataConnectionType ${dataConnectionType} is already registered. Unable to register another manager.`
+      );


does this error crash the server?

Yes. This error will crash the server, the same as when registering the same route multiple times.

CHANGELOG.md

github-actions · 2025-04-30T22:29:31Z

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "feat". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

joshuali925 · 2025-04-30T23:09:49Z

src/plugins/query_enhancements/server/routes/resources/routes.ts

+    {
+      path: `${BASE_API}/resources`,
+      validate: {
+        query: schema.object({}, { unknowns: 'allow' }),


i don't think it needs query if it's already using content?

Do we want to keep the ability, so that some handlers can still use queryParams?

Removed the query. If other handlers want to use queryParams, they can request to update it later.

joshuali925 · 2025-04-30T23:10:01Z

src/plugins/query_enhancements/server/routes/resources/routes.ts

+            name: schema.maybe(schema.string()),
+            type: schema.string(),
+          }),
+          content: schema.maybe(schema.any()),


should this be schema.object({}, { unknowns: 'allow' })?

Updated. Thank you!

codecov · 2025-04-30T23:15:58Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 56.76%. Comparing base (a37c8ea) to head (2bd2b10).
Report is 4 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (a37c8ea) and HEAD (2bd2b10). Click for more details.

HEAD has 2 uploads less than BASE

Flag BASE (a37c8ea) HEAD (2bd2b10)

Linux_4 1 0

Windows_4 1 0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9731      +/-   ##
==========================================
- Coverage   61.91%   56.76%   -5.15%     
==========================================
  Files        3826     3387     -439     
  Lines       92552    84491    -8061     
  Branches    14740    13490    -1250     
==========================================
- Hits        57301    47965    -9336     
- Misses      31547    33474    +1927     
+ Partials     3704     3052     -652

Flag	Coverage Δ
Linux_1	`28.89% <ø> (ø)`
Linux_2	`56.43% <ø> (ø)`
Linux_3	`39.57% <ø> (ø)`
Linux_4	`?`
Windows_1	`28.91% <ø> (ø)`
Windows_2	`56.39% <ø> (ø)`
Windows_3	`39.59% <ø> (+<0.01%)`	⬆️
Windows_4	`?`

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.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2025-04-30T23:24:15Z

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "Add resource API pattern in query_enhancements". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

joshuali925 · 2025-05-02T19:17:26Z

src/plugins/query_enhancements/server/routes/resources/routes.ts

+        return response.notFound();
+      }
+      try {
+        const resourcesResponse = await manager.getResources(context, request);


i just noticed that this is still getResources. with this how would you create a resource? while you may pass in arbitrary operations as resource type, it won't make sense if the manager call is always getResources

it might be better to make the path ${BASE_API}/resources/search (i would prefer to have a separate POST route for search, rather than putting get, list, create in the same route), or change this to handlePostRequest rather than getResources, in order to distinguish between get and create

Signed-off-by: Kathy Guo <[email protected]>

github-actions bot added the first-time-contributor label Apr 25, 2025

github-actions bot added the failed changeset label Apr 25, 2025

kavilla reviewed Apr 25, 2025

View reviewed changes

src/plugins/query_enhancements/server/routes/resources/routes.ts Outdated Show resolved Hide resolved

kavilla reviewed Apr 25, 2025

View reviewed changes

src/plugins/query_enhancements/server/routes/resources/routes.ts Outdated Show resolved Hide resolved

joshuali925 reviewed Apr 29, 2025

View reviewed changes

kathyguo18 force-pushed the feature/resource branch 2 times, most recently from 2a1778c to a112aac Compare April 30, 2025 22:22

joshuali925 reviewed Apr 30, 2025

View reviewed changes

kathyguo18 requested review from joshuali925 and kavilla April 30, 2025 23:18

kathyguo18 force-pushed the feature/resource branch from a112aac to 2bd2b10 Compare April 30, 2025 23:32

joshuali925 reviewed May 2, 2025

View reviewed changes

kathyguo18 force-pushed the feature/resource branch from 2bd2b10 to 65efa1d Compare May 5, 2025 20:14

Add resource API pattern

5483e45

Signed-off-by: Kathy Guo <[email protected]>

kathyguo18 force-pushed the feature/resource branch from 42da8e7 to 5483e45 Compare May 6, 2025 15:31

joshuali925 mentioned this pull request May 6, 2025

Add resource API pattern #9770

Open

7 tasks

kathyguo18 closed this May 7, 2025

joshuali925 mentioned this pull request May 8, 2025

[RFC] Add A Resource API in Query Enhancements #9779

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add resource API pattern #9731

Add resource API pattern #9731

kathyguo18 commented Apr 25, 2025 •

edited

Loading

github-actions bot commented Apr 25, 2025

joshuali925 Apr 25, 2025

kathyguo18 Apr 30, 2025

joshuali925 Apr 25, 2025

kathyguo18 Apr 30, 2025

github-actions bot commented Apr 30, 2025

joshuali925 Apr 30, 2025

kathyguo18 Apr 30, 2025

kathyguo18 Apr 30, 2025

joshuali925 Apr 30, 2025

kathyguo18 Apr 30, 2025

codecov bot commented Apr 30, 2025 •

edited

Loading

github-actions bot commented Apr 30, 2025

joshuali925 May 2, 2025

		registerResourceManager: (dataConnectionType: string, manager: BaseConnectionManager) =>
		resourceManagerService.register(dataConnectionType, manager),

	registerResourceManager: (dataConnectionType: string, manager: BaseConnectionManager) =>
	resourceManagerService.register(dataConnectionType, manager),
	registerResourceManager: resourceManagerService.register,

Add resource API pattern #9731

Add resource API pattern #9731

Conversation

kathyguo18 commented Apr 25, 2025 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Changelog

Check List

github-actions bot commented Apr 25, 2025

ℹ️ Manual Changeset Creation Reminder

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 30, 2025

❌ Invalid Prefix For Manual Changeset Creation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 30, 2025 • edited Loading

Codecov Report

github-actions bot commented Apr 30, 2025

❌ Invalid Prefix For Manual Changeset Creation

Choose a reason for hiding this comment

kathyguo18 commented Apr 25, 2025 •

edited

Loading

codecov bot commented Apr 30, 2025 •

edited

Loading