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

Initial opensearch backed v2 endpoint #511

Merged
merged 1 commit into from
Apr 22, 2022
Merged

Initial opensearch backed v2 endpoint #511

merged 1 commit into from
Apr 22, 2022

Conversation

JPrevost
Copy link
Member

Why are these changes being introduced:

  • Being able to connect to OpenSearch from TIMDEX API is a building
    block towards implementing the v2 API

Relevant ticket(s):

How does this address that need:

  • creates a toggleable v2 version of the v1 endpoints
  • passes clients into the endpoints to allow v1 and v2 to use different
    clients while (for now) sharing most of the querying logic

Document any side effects to this change:

  • DockerFile, composer, and makefile are not yet updated to reflect
    being backed by both ES and OS at the same time.
  • GraphQL is only available on the V1 endpoint
  • The v2 endpoint this introduces is largely the same spec as v1 and
    does not reflect the future needs of the v2 endpoint. It essentially
    allows v1-like functionality to be backed by opensearch. We have
    additional tickets open to plan and implement the actual v2 endpoint.

Requires Database Migrations?

NO

Includes new or updated dependencies?

YES

@JPrevost JPrevost requested a review from jazairi April 20, 2022 19:45
@jazairi jazairi self-assigned this Apr 20, 2022
Copy link
Contributor

@jazairi jazairi left a comment

Choose a reason for hiding this comment

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

I think this approach makes sense, and it should make it easier for us to deprecate v1 should we want to do that down the road. I don't think that some of the CodeClimate suggestions are applicable here (e.g., the DRY warnings), but I would recommend cleaning up the minor stuff (trailing whitespace, empty lines, instance_of?, parenthesis) before we merge this.

Why are these changes being introduced:

* Being able to connect to OpenSearch from TIMDEX API is a building
  block towards implementing the v2 API

Relevant ticket(s):

* https://mitlibraries.atlassian.net/browse/RDI-27

How does this address that need:

* creates a toggleable v2 version of the v1 endpoints
* passes clients into the endpoints to allow v1 and v2 to use different
  clients while (for now) sharing most of the querying logic

Document any side effects to this change:

* DockerFile, composer, and makefile are not yet updated to reflect
  being backed by both ES and OS at the same time.
* GraphQL is only available on the V1 endpoint
* The v2 endpoint this introduces is largely the same spec as v1 and
  does not reflect the future needs of the v2 endpoint. It essentially
  allows v1-like functionality to be backed by opensearch. We have
  additional tickets open to plan and implement the actual v2 endpoint.
@JPrevost JPrevost requested a review from jazairi April 22, 2022 13:25
@JPrevost JPrevost merged commit 0be55aa into main Apr 22, 2022
@JPrevost JPrevost deleted the opensearch branch April 22, 2022 13:26
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.

2 participants