-
Notifications
You must be signed in to change notification settings - Fork 722
OpenSearch Support #880
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
OpenSearch Support #880
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
kukushking
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good - left a few comments, mostly minor
tests/test_opensearch.py
Outdated
| logging.getLogger("awswrangler").setLevel(logging.DEBUG) | ||
|
|
||
| # TODO: create test_infra for opensearch | ||
| OPENSEARCH_DOMAIN = 'search-es71-public-z63iyqxccc4ungar5vx45xwgfi.us-east-1.es.amazonaws.com' # change to your domain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually use pytest fixtures that fetch the values from CloudFormation outputs, example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this was just temporary till I finish the test_infra which I now have.
checkout these files:
- test_infra/scripts/delete-opensearch.sh
- test_infra/scripts/deploy-opensearch.sh
- test_infra/stacks/opensearch_stack.py
- test_infra/app.py
- tests/_utils.py
- tests/test_opensearch.py
…nto elasticsearch-support
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
…nto elasticsearch-support
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
@AssafMentzer I updated the role and reran the tests - results should be here soon |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
maxispeicher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| return client.info().get("version", {}).get("number") | ||
|
|
||
|
|
||
| def _get_version_major(client: OpenSearch) -> Any: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed for anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes in opensearch/_write.py create_index method to support the deprecation of ES mapping types
| "aws-cdk.aws-rds" = "^1.115.0" | ||
| "aws-cdk.aws-secretsmanager" = "^1.115.0" | ||
| "aws-cdk.aws-ssm" = "^1.115.0" | ||
| "aws-cdk.aws-opensearchservice" = "^1.124.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all cdk packages should have the same minimum version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point. made the change: commit
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Issue #, if available:
Elasticsearch support
Description of changes:
initialize code: only structure and _write functions signatures
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.