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

Add point in time api #253

Merged
merged 6 commits into from
Mar 24, 2023
Merged

Conversation

Jakob3xD
Copy link
Collaborator

Description

Add the Point in Time API.
https://opensearch.org/docs/latest/search-plugins/point-in-time-api/

Issues Resolved

Closes #251

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.

@Jakob3xD
Copy link
Collaborator Author

Jakob3xD commented Mar 20, 2023

ToDo:

  • Add some tests
  • Update comments

@Jakob3xD Jakob3xD force-pushed the feature-poin-in-time branch 2 times, most recently from 1b09f7a to f9f678b Compare March 23, 2023 15:34
@Jakob3xD
Copy link
Collaborator Author

@dblock does this lib aim to be a low level lib or should it be more user friendly?
I am asking because I added some custom Response types for Point In Time requests and also avoid the user to build the delete body.

Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com>
@Jakob3xD Jakob3xD force-pushed the feature-poin-in-time branch from f9f678b to 98b665e Compare March 23, 2023 17:08
@dblock
Copy link
Member

dblock commented Mar 23, 2023

@dblock does this lib aim to be a low level lib or should it be more user friendly? I am asking because I added some custom Response types for Point In Time requests and also avoid the user to build the delete body.

I definitely think we want to minimize the amount of work a user has to do. So as much user friendly as possible, but also letting users go deep into low level constructs.

@Jakob3xD Jakob3xD force-pushed the feature-poin-in-time branch 2 times, most recently from 238981c to 8f56714 Compare March 24, 2023 11:03
Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com>
Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com>
Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com>
Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com>
@Jakob3xD Jakob3xD force-pushed the feature-poin-in-time branch from 8f56714 to f334868 Compare March 24, 2023 11:15
Signed-off-by: Jakob Hahn <jakob.hahn@hetzner.com>
@Jakob3xD Jakob3xD marked this pull request as ready for review March 24, 2023 11:23
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

Small nit.

// Modifications Copyright OpenSearch Contributors. See
// GitHub history for details.

// Licensed to Elasticsearch B.V. under one or more contributor
Copy link
Member

Choose a reason for hiding this comment

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

This is new code? We don't need a ES grant, remove this license part below.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Question for me as I am not into such license things.
Can I remove the ES grant if it is new code but follows the same scheme and has similarity to code that has ES grant? As the Withxyz() functions are all the same.

Copy link
Member

Choose a reason for hiding this comment

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

IANAL, but if you copied any code with such license then we should probably keep it. But if it's new code then we don't need it. Check out https://github.com/opensearch-project/.github/blob/main/CONTRIBUTING.md#license-headers for more info.

@dblock dblock merged commit 0da56b2 into opensearch-project:main Mar 24, 2023
@Jakob3xD Jakob3xD deleted the feature-poin-in-time branch March 24, 2023 15:46
VachaShah added a commit to VachaShah/opensearch-go that referenced this pull request May 18, 2023
…e response.Err()

Signed-off-by: Vacha Shah <vachshah@amazon.com>
dblock pushed a commit that referenced this pull request May 22, 2023
#322)

* Revert "Draft: Add Err() function to Response for detailed errors (#246)"

This reverts commit 8d3cf4d.

Signed-off-by: Vacha Shah <vachshah@amazon.com>

* Fixing PIT APIs introduced in PR #253 since they use response.Err()

Signed-off-by: Vacha Shah <vachshah@amazon.com>

* Removing opensearchapi.Error usage from snapshot tests introduced in PR #237

Signed-off-by: Vacha Shah <vachshah@amazon.com>

* Resolve Changelog conflicts

Signed-off-by: Vacha Shah <vachshah@amazon.com>

---------

Signed-off-by: Vacha Shah <vachshah@amazon.com>
VachaShah added a commit to VachaShah/opensearch-go that referenced this pull request Oct 3, 2023
opensearch-project#322)

* Revert "Draft: Add Err() function to Response for detailed errors (opensearch-project#246)"

This reverts commit 8d3cf4d.

Signed-off-by: Vacha Shah <vachshah@amazon.com>

* Fixing PIT APIs introduced in PR opensearch-project#253 since they use response.Err()

Signed-off-by: Vacha Shah <vachshah@amazon.com>

* Removing opensearchapi.Error usage from snapshot tests introduced in PR opensearch-project#237

Signed-off-by: Vacha Shah <vachshah@amazon.com>

* Resolve Changelog conflicts

Signed-off-by: Vacha Shah <vachshah@amazon.com>

---------

Signed-off-by: Vacha Shah <vachshah@amazon.com>
dblock pushed a commit that referenced this pull request Oct 4, 2023
#322) (#384)

* Revert "Draft: Add Err() function to Response for detailed errors (#246)"

This reverts commit 8d3cf4d.



* Fixing PIT APIs introduced in PR #253 since they use response.Err()



* Removing opensearchapi.Error usage from snapshot tests introduced in PR #237



* Resolve Changelog conflicts



---------

Signed-off-by: Vacha Shah <vachshah@amazon.com>
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.

[FEATURE] Add point_in_time API
2 participants