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

Create index atbd if not exists on opensearch client setup #699

Merged
merged 1 commit into from
Apr 24, 2023

Conversation

thenav56
Copy link
Collaborator

@thenav56 thenav56 commented Apr 18, 2023

Related to

Alternative option

Run this on app startup

diff --git a/app/main.py b/app/main.py
index d341927..63f8e87 100644
--- a/app/main.py
+++ b/app/main.py
@@ -37,3 +37,15 @@ app.include_router(api_router, prefix=config.API_VERSION_STRING)
 def ping():
     """Health check."""
     return {"ping": "pong!"}
+
+
+@app.on_event("startup")
+def create_search_indices():
+    """Create index in opensearch if not exists already."""
+    from app.search.opensearch import (
+        aws_auth,
+        create_search_indices as o_create_search_indices,
+    )
+
+    opensearch_client = aws_auth()
+    o_create_search_indices(opensearch_client)
diff --git a/docker-compose.yml b/docker-compose.yml
index afcfcbf..ff0fe32 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -130,6 +130,7 @@ services:
       PDF_PREVIEW_HOST: http://ui:9000
     depends_on:
       - bootstrapper
+      - opensearch
 
   worker:
     build:

Why not used in this PR: Adding this change will need opensearch to be always running in development and opensearch container is a bit heavy compared to others on resource usage.

@thenav56 thenav56 force-pushed the fix/es-create-index branch 2 times, most recently from 168d5e3 to ec3663b Compare April 18, 2023 10:34
@thenav56 thenav56 marked this pull request as ready for review April 18, 2023 11:24
@thenav56 thenav56 requested a review from sunu April 18, 2023 15:15
Copy link
Collaborator

@sunu sunu left a comment

Choose a reason for hiding this comment

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

Thanks so much for the fix @thenav56! I think it's a bit unconventional in terms of placement (i.e. I wouldn't normally expect that calling a function named aws_auth will create a search index for me) but I wouldn't consider it a blocker.

I would suggest that once we figure out a way to implement #452, we should move the es index creation to that mechanism so that postgres migrations and es index creation runs through the same mechanism allowing the index to be created during deployment instead of at runtime

@sunu sunu merged commit fccc965 into develop Apr 24, 2023
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