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

CatIndexTool implementation #1582

Merged

Conversation

dbwiddis
Copy link
Member

@dbwiddis dbwiddis commented Nov 2, 2023

Description

Implements Cat Index Tool with edge case handling and tests

Issues Resolved

Fixes #1547

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@dbwiddis dbwiddis had a problem deploying to ml-commons-cicd-env November 2, 2023 01:22 — with GitHub Actions Failure
@dbwiddis dbwiddis had a problem deploying to ml-commons-cicd-env November 2, 2023 01:22 — with GitHub Actions Failure
@dbwiddis dbwiddis had a problem deploying to ml-commons-cicd-env November 2, 2023 01:26 — with GitHub Actions Failure
@dbwiddis dbwiddis had a problem deploying to ml-commons-cicd-env November 2, 2023 01:26 — with GitHub Actions Failure
@dbwiddis dbwiddis had a problem deploying to ml-commons-cicd-env November 2, 2023 01:26 — with GitHub Actions Failure
@dbwiddis dbwiddis had a problem deploying to ml-commons-cicd-env November 2, 2023 01:37 — with GitHub Actions Failure
@dbwiddis dbwiddis had a problem deploying to ml-commons-cicd-env November 2, 2023 01:37 — with GitHub Actions Failure
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 2, 2023 04:05 — with GitHub Actions Inactive
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 2, 2023 04:05 — with GitHub Actions Inactive
@dbwiddis dbwiddis had a problem deploying to ml-commons-cicd-env November 2, 2023 04:05 — with GitHub Actions Failure
@dbwiddis
Copy link
Member Author

dbwiddis commented Nov 2, 2023

Test failures unassociated with this PR:

46 tests completed, 1 failed, 9 skipped
Tests with failures:
 - org.opensearch.ml.rest.RestMLDeployModelActionIT.testReDeployModel

Tests pass locally:

➜  ml-commons git:(cat-index) ./gradlew :opensearch-ml-algorithms:test --tests 'CatIndexToolTests'   
=======================================
OpenSearch Build Hamster says Hello!
  Gradle Version        : 8.1.1
  OS Info               : Mac OS X 13.6 (x86_64)
  JDK Version           : 17 (Amazon Corretto JDK)
  JAVA_HOME             : /Library/Java/JavaVirtualMachines/amazon-corretto-17.jdk/Contents/Home
  Random Testing Seed   : BBF6D34AA5D9DD5B
  In FIPS 140 mode      : false
=======================================

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/8.1.1/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 5s
11 actionable tasks: 1 executed, 10 up-to-date

if (parameters.containsKey("indices")) {
@SuppressWarnings("unchecked")
List<String> indexList = gson.fromJson(parameters.get("indices"), List.class);
indices = indexList.toArray(new String[0]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

what happens if a request comes to cat an system index?

Copy link
Member Author

Choose a reason for hiding this comment

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

Huh. Looks like I need to use the getSettings approach here

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed to match REST API

@dbwiddis dbwiddis had a problem deploying to ml-commons-cicd-env November 4, 2023 20:09 — with GitHub Actions Failure
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 4, 2023 20:09 — with GitHub Actions Inactive
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 4, 2023 20:09 — with GitHub Actions Inactive
@dbwiddis dbwiddis requested a review from dhrubo-os November 4, 2023 20:10
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 4, 2023 20:24 — with GitHub Actions Inactive
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 8, 2023 00:10 — with GitHub Actions Inactive
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 8, 2023 00:10 — with GitHub Actions Inactive
Signed-off-by: Daniel Widdis <[email protected]>
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 8, 2023 01:33 — with GitHub Actions Inactive
@dbwiddis dbwiddis had a problem deploying to ml-commons-cicd-env November 8, 2023 01:33 — with GitHub Actions Failure
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 8, 2023 01:33 — with GitHub Actions Inactive
Signed-off-by: Daniel Widdis <[email protected]>
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 8, 2023 07:00 — with GitHub Actions Inactive
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 8, 2023 07:00 — with GitHub Actions Inactive
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 8, 2023 07:00 — with GitHub Actions Inactive
@dbwiddis dbwiddis had a problem deploying to ml-commons-cicd-env November 8, 2023 07:00 — with GitHub Actions Failure
@dbwiddis dbwiddis mentioned this pull request Nov 8, 2023
5 tasks
Signed-off-by: Daniel Widdis <[email protected]>
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 8, 2023 16:09 — with GitHub Actions Inactive
@dbwiddis dbwiddis had a problem deploying to ml-commons-cicd-env November 8, 2023 16:09 — with GitHub Actions Failure
@dbwiddis dbwiddis temporarily deployed to ml-commons-cicd-env November 8, 2023 16:10 — with GitHub Actions Inactive
@dhrubo-os dhrubo-os merged commit 7d1741c into opensearch-project:feature/agent_framework_dev Nov 8, 2023
4 of 7 checks passed
@dbwiddis dbwiddis deleted the cat-index branch November 8, 2023 20:29
arjunkumargiri pushed a commit to arjunkumargiri/ml-commons that referenced this pull request Nov 10, 2023
Updated version of awssdk (opensearch-project#1605) (opensearch-project#1606)

Signed-off-by: Owais Kazi <[email protected]>
(cherry picked from commit 7e44eb2)

Co-authored-by: Owais Kazi <[email protected]>

create agent

Signed-off-by: Jing Zhang <[email protected]>

CatIndexTool implementation (opensearch-project#1582)

* CatIndexTool implementation

Signed-off-by: Daniel Widdis <[email protected]>

* Match implementation to REST API to respect index permissions

Signed-off-by: Daniel Widdis <[email protected]>

* Javadoc fixes

Signed-off-by: Daniel Widdis <[email protected]>

* Reduce fields to exactly match _cat/indices

Signed-off-by: Daniel Widdis <[email protected]>

* Add TODO, clean up unused code/imports

Signed-off-by: Daniel Widdis <[email protected]>

* Rebase with upstream

Signed-off-by: Daniel Widdis <[email protected]>

* Remove alias getters/setters accidentally kept when rebasing

Signed-off-by: Daniel Widdis <[email protected]>

* Update test json format to list

Signed-off-by: Daniel Widdis <[email protected]>

* Remove unused modelId

Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Arjun kumar Giri <[email protected]>
ylwu-amzn added a commit that referenced this pull request Nov 14, 2023
#1611)

* Add support for including intermediay tool response to agent response

Signed-off-by: Arjun kumar Giri <[email protected]>

* Add include intermediary response support to react agent runner

Signed-off-by: Arjun kumar Giri <[email protected]>

* Rebase from upstream

Updated version of awssdk (#1605) (#1606)

Signed-off-by: Owais Kazi <[email protected]>
(cherry picked from commit 7e44eb2)

Co-authored-by: Owais Kazi <[email protected]>

create agent

Signed-off-by: Jing Zhang <[email protected]>

CatIndexTool implementation (#1582)

* CatIndexTool implementation

Signed-off-by: Daniel Widdis <[email protected]>

* Match implementation to REST API to respect index permissions

Signed-off-by: Daniel Widdis <[email protected]>

* Javadoc fixes

Signed-off-by: Daniel Widdis <[email protected]>

* Reduce fields to exactly match _cat/indices

Signed-off-by: Daniel Widdis <[email protected]>

* Add TODO, clean up unused code/imports

Signed-off-by: Daniel Widdis <[email protected]>

* Rebase with upstream

Signed-off-by: Daniel Widdis <[email protected]>

* Remove alias getters/setters accidentally kept when rebasing

Signed-off-by: Daniel Widdis <[email protected]>

* Update test json format to list

Signed-off-by: Daniel Widdis <[email protected]>

* Remove unused modelId

Signed-off-by: Daniel Widdis <[email protected]>

---------

Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: Arjun kumar Giri <[email protected]>

* Revert "Rebase from upstream"

This reverts commit 49cff61.

Signed-off-by: Arjun kumar Giri <[email protected]>

* Add unit tests

Signed-off-by: Arjun kumar Giri <[email protected]>

* Rebase upstream changes

Signed-off-by: Arjun kumar Giri <[email protected]>

* Fixed PR feedback

Signed-off-by: Arjun kumar Giri <[email protected]>

---------

Signed-off-by: Arjun kumar Giri <[email protected]>
Signed-off-by: Daniel Widdis <[email protected]>
Signed-off-by: arjunkumargiri <[email protected]>
Co-authored-by: Arjun kumar Giri <[email protected]>
Co-authored-by: Yaliang Wu <[email protected]>
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.

3 participants