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

Configure plugin for calling Search API #20

Merged
merged 4 commits into from
Dec 12, 2023

Conversation

ronitagarwala01
Copy link
Contributor

@ronitagarwala01 ronitagarwala01 commented Nov 21, 2023

Add function to call a post-deployment search API file defined in gcn.nasa.gov. Allow for execution of REST API calls in both sandbox and production environments.

data.ts Outdated Show resolved Hide resolved
index.ts Outdated Show resolved Hide resolved
index.ts Outdated Show resolved Hide resolved
index.ts Outdated Show resolved Hide resolved
@ronitagarwala01 ronitagarwala01 force-pushed the nlp_config branch 2 times, most recently from 7439adc to be574d8 Compare November 26, 2023 04:46
Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

@ronitagarwala01
Copy link
Contributor Author

ronitagarwala01 commented Nov 27, 2023

You mean import the client configured by architect-functions-search? I haven't tried that yet. Let me see if it works.

@lpsinger
Copy link
Member

You mean import the client configured by architect-functions-search? I haven't tried that yet. Let me see if it works.

Yes, exactly. If the service discovery is working, then architect-functions-search should work too.

@ronitagarwala01
Copy link
Contributor Author

You were right. It works!

index.ts Outdated Show resolved Hide resolved
index.ts Outdated Show resolved Hide resolved
index.ts Outdated Show resolved Hide resolved
index.ts Outdated Show resolved Hide resolved
index.ts Outdated Show resolved Hide resolved
index.ts Outdated Show resolved Hide resolved
@ronitagarwala01 ronitagarwala01 force-pushed the nlp_config branch 2 times, most recently from dbc8d6d to c10538c Compare December 4, 2023 17:09
@lpsinger
Copy link
Member

lpsinger commented Dec 4, 2023

Please rebase.

@ronitagarwala01 ronitagarwala01 force-pushed the nlp_config branch 2 times, most recently from cbc84f8 to f5e59c1 Compare December 4, 2023 21:16
@ronitagarwala01 ronitagarwala01 changed the title Configure plugin for NLP Search Configure plugin for calling OpenSearch API Dec 4, 2023
@ronitagarwala01
Copy link
Contributor Author

Done.

Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

Please document this feature in the README file.

index.ts Outdated Show resolved Hide resolved
index.ts Outdated Show resolved Hide resolved
index.ts Outdated Show resolved Hide resolved
@ronitagarwala01 ronitagarwala01 changed the title Configure plugin for calling OpenSearch API Configure plugin for calling Search API Dec 7, 2023
@ronitagarwala01
Copy link
Contributor Author

Updated README.md

README.md Outdated Show resolved Hide resolved
index.ts Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Execute post-deployment API requests in both sandbox mode and production.
Add @nasa-gcn/architect-functions-search as dependency.
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
index.ts Outdated Show resolved Hide resolved
ronitagarwala01 and others added 3 commits December 11, 2023 22:02
Copy link
Member

@lpsinger lpsinger left a comment

Choose a reason for hiding this comment

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

Looks great!

Side note: isn't it ironic that we overrode deploy.start for production but sandbox.end for local development? Too bad the plugin entry point names are not more consistent.

@lpsinger lpsinger merged commit 80196c3 into nasa-gcn:main Dec 12, 2023
@ronitagarwala01
Copy link
Contributor Author

I know right? It makes it a little confusing imo, but there doesn't seem to be a work around.

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