Skip to content

Conversation

@jherns
Copy link
Contributor

@jherns jherns commented Feb 10, 2025

  • Add support for following an index
  • Update test framework to support CCR
    • will skip CCR tests if the replica isn't reachable
  • Update readme

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This pull request adds support for following an index using the PUT _ccr/follow API and updates test infrastructure and documentation for CCR.

  • Adds a new CCR client module with a follow method in the client library
  • Introduces tests for following a leader index and sets up a replica client in the test helper
  • Updates the README with CCR testing instructions

Changes

File Description
test/client/ccr_test.rb Adds tests to verify following a leader index using CCR
lib/elastomer_client/client/ccr.rb Introduces the new CCR module and the follow method implementation
test/test_helper.rb Configures the replica client for CCR tests
README.md Updates usage instructions for CCR tests

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

#
# See https://www.elastic.co/guide/en/elasticsearch/reference/current/ccr-put-follow.html
def follow(follower_index, body, params = {})
response = client.put "/#{follower_index}/_ccr/follow", params.merge(params, body:, action: "follow", rest_api: "ccr")
Copy link

Copilot AI Feb 10, 2025

Choose a reason for hiding this comment

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

The use of params.merge(params, ...) appears redundant and may lead to unexpected behavior. Consider simplifying it to params.merge(body: body, action: "follow", rest_api: "ccr") to avoid merging the same hash twice.

Suggested change
response = client.put "/#{follower_index}/_ccr/follow", params.merge(params, body:, action: "follow", rest_api: "ccr")
response = client.put "/#{follower_index}/_ccr/follow", params.merge(body: body, action: "follow", rest_api: "ccr")

Copilot uses AI. Check for mistakes.
# ccr.follow("follower_index", { leader_index: "leader_index", remote_cluster: "leader" })
#
# See https://www.elastic.co/guide/en/elasticsearch/reference/current/ccr-put-follow.html
def follow(follower_index, body, params = {})
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we at least check to make sure the following index was created here?

{ "follow_index_created" : true }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that is on the user (elastomer) to confirm the index was created.

@jherns jherns merged commit 586f77e into main Feb 11, 2025
5 checks passed
@jherns jherns deleted the jherns/follow branch February 11, 2025 17:19
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.

4 participants