Skip to content
This repository was archived by the owner on Nov 7, 2025. It is now read-only.

Conversation

@nablaone
Copy link
Member

@nablaone nablaone commented Jun 17, 2025

This PR changes the method by which we store virtual table definitions in Elasticsearch. It fixes 409 errors that can occur during write.

@nablaone nablaone marked this pull request as ready for review June 18, 2025 13:32
@nablaone nablaone requested a review from a team as a code owner June 18, 2025 13:32
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.

Pull Request Overview

This PR updates how virtual table definitions are stored in Elasticsearch by switching from the _update endpoint with upsert logic to the _doc endpoint using PUT, eliminating 409 conflicts on writes.

  • Change write URL from /_update/{id} to /_doc/{id}
  • Remove JSON upsert wrapper and simplify payload to the Wrapper struct
  • Switch HTTP method from POST to PUT for document creation/replacement
Comments suppressed due to low confidence (1)

platform/persistence/elastic.go:62

  • Add unit tests for the new PUT _doc behavior to verify that documents are created or replaced correctly and that 409 conflicts no longer occur.
	elasticsearchURL := fmt.Sprintf("%s/_doc/%s", p.indexName, key)

}

resp, err := p.httpClient.Request(context.Background(), "POST", elasticsearchURL, jsonData)
resp, err := p.httpClient.Request(context.Background(), "PUT", elasticsearchURL, jsonData)
Copy link

Copilot AI Jun 18, 2025

Choose a reason for hiding this comment

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

The response status is not checked after the request. Consider verifying resp.StatusCode for non-2xx values and returning an error for unexpected statuses to surface server-side failures.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

It's checked in line 77. Please review again.

@nablaone nablaone added this pull request to the merge queue Jun 18, 2025
Merged via the queue into main with commit 135fadc Jun 18, 2025
6 checks passed
@nablaone nablaone deleted the que-275-error-storing-virtual-table-failed-to-elastic-409-conflict branch June 18, 2025 15:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants