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

removing api keys from the integ test log #3112

Merged
merged 4 commits into from
Oct 17, 2024

Conversation

dhrubo-os
Copy link
Collaborator

Description

[removing api keys from the integ test log

Solving this issue: #2915

Current exception should be:

testDeleteConnector FAILED org.opensearch.client.ResponseException: method [POST], host [http://127.0.0.1:55810], URI [/_plugins/_ml/connectors/_create], status line [HTTP/1.1 503 Service Unavailable] {"error":{"root_cause":[{"type":"unavailable_shards_exception","reason":"[.plugins-ml-connector][0] primary shard is not active Timeout: [1m], request: [BulkShardRequest [[.plugins-ml-connector][0]] containing [index {[.plugins-ml-connector][bnO6yZEB1QdOTLrBQkCP], source[{"name":"OpenAI Connector","version":"1","description":"The connector to public OpenAI model service for GPT 3.5","protocol":"http","parameters":{"endpoint":"api.openai.com","content_type":"application/json","auth":"API_Key","max_tokens":"7","temperature":"0","model":"gpt-3.5-turbo-instruct"},"credential": "***","actions":[{"action_type":"PREDICT","method":"POST","url":"https://${parameters.endpoint}/v1/completions","headers":{"Authorization":"Bearer ${credential.openAI_key}"},"request_body":"{ \"model\": \"${parameters.model}\", \"prompt\": \"${parameters.prompt}\",  \"max_tokens\": ${parameters.max_tokens},  \"temperature\": ${parameters.temperature} }"}],"client_config":{"max_connection":20,"connection_timeout":50000,"read_timeout":50000,"retry_backoff_millis":200,"retry_timeout_seconds":30,"max_retry_times":0,"retry_backoff_policy":"constant"}}]}] and a refresh]"}],"type":"unavailable_shards_exception","reason":"[.plugins-ml-connector][0] primary shard is not active Timeout: [1m], request: [BulkShardRequest [[.plugins-ml-connector][0]] containing [index {[.plugins-ml-connector][bnO6yZEB1QdOTLrBQkCP], source[{"name":"OpenAI Connector","version":"1","description":"The connector to public OpenAI model service for GPT 3.5","protocol":"http","parameters":{"endpoint":"api.openai.com","content_type":"application/json","auth":"API_Key","max_tokens":"7","temperature":"0","model":"gpt-3.5-turbo-instruct"},"credential": "***","actions":[{"action_type":"PREDICT","method":"POST","url":"https://${parameters.endpoint}/v1/completions","headers":{"Authorization":"Bearer ${credential.openAI_key}"},"request_body":"{ \"model\": \"${parameters.model}\", \"prompt\": \"${parameters.prompt}\",  \"max_tokens\": ${parameters.max_tokens},  \"temperature\": ${parameters.temperature} }"}],"client_config":{"max_connection":20,"connection_timeout":50000,"read_timeout":50000,"retry_backoff_millis":200,"retry_timeout_seconds":30,"max_retry_times":0,"retry_backoff_policy":"constant"}}]}] and a refresh]"},"status":503}

]

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • New functionality includes testing.
  • New functionality has been documented.
  • API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • Public documentation issue/PR created.

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.

Signed-off-by: Dhrubo Saha <[email protected]>
@austintlee
Copy link
Collaborator

Don't you need to do this for all integ tests that use connectors?

Signed-off-by: Dhrubo Saha <[email protected]>
@dhrubo-os
Copy link
Collaborator Author

Don't you need to do this for all integ tests that use connectors?

Yeah, agree. Now I added that try catch block in the createConnector method as we most likely get the exception output from this method.


}

private static String maskSensitiveInfo(String input) {
Copy link
Contributor

@pyek-bot pyek-bot Oct 16, 2024

Choose a reason for hiding this comment

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

minor: can add a simple test case for this

}

public void testGetConnector() throws IOException {
public void testCreate_Get_DeleteConnector() throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

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

since we have combined the 3 api calls, does the error message give enough information to validate at which step (create, get, delete) the test failed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If any assertion fails, it will indicate which assertion is failing within the method, correct? I’ve noticed a lot of redundant resource creation in this integration test class, which I plan to remove step by step. In this case, we are now creating the connector resource once and reusing it for three tests, rather than creating the resource three separate times.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, i misunderstood the change - assumed it was a connection test to the connector itself during creation phase via API and not integ tests. Thanks for clearing this up!

austintlee
austintlee previously approved these changes Oct 16, 2024
@dhrubo-os dhrubo-os merged commit bc030fa into opensearch-project:main Oct 17, 2024
8 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Oct 17, 2024
* removing api keys from the integ test log

Signed-off-by: Dhrubo Saha <[email protected]>

* apply spotless

Signed-off-by: Dhrubo Saha <[email protected]>

* addressed comment

Signed-off-by: Dhrubo Saha <[email protected]>

* adding unit test to address comments

Signed-off-by: Dhrubo Saha <[email protected]>

---------

Signed-off-by: Dhrubo Saha <[email protected]>
(cherry picked from commit bc030fa)
dhrubo-os added a commit that referenced this pull request Oct 17, 2024
* removing api keys from the integ test log

Signed-off-by: Dhrubo Saha <[email protected]>

* apply spotless

Signed-off-by: Dhrubo Saha <[email protected]>

* addressed comment

Signed-off-by: Dhrubo Saha <[email protected]>

* adding unit test to address comments

Signed-off-by: Dhrubo Saha <[email protected]>

---------

Signed-off-by: Dhrubo Saha <[email protected]>
(cherry picked from commit bc030fa)

Co-authored-by: Dhrubo Saha <[email protected]>
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.

4 participants