Skip to content

Add Search Applications API #2082

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

Merged
merged 9 commits into from
Apr 19, 2023
Merged

Conversation

carlosdelest
Copy link
Member

Add Search Applications API, including both CRUD and _search request.

Reopening this from #2080, as Github actions need the PR to be done from the same repo

@github-actions
Copy link
Contributor

Following you can find the validation results for the APIs you have changed.

API Status Request Response
search_application.delete_behavioral_analytics 🟠 Missing type Missing type
search_application.delete Missing test Missing test
search_application.get_behavioral_analytics 🟠 Missing type Missing type
search_application.get Missing test Missing test
search_application.list Missing test Missing test
search_application.post_behavioral_analytics_event 🟠 Missing type Missing type
search_application.put_behavioral_analytics 🟠 Missing type Missing type
search_application.put Missing test Missing test
search_application.search Missing test Missing test

You can validate these APIs yourself by using the make validate target.

Copy link
Member

@kderusso kderusso 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 so far, a couple comments! 💯

"responseMediaType": [
"application/json"
],
"since": "8.8.0",
"stability": "beta",
Copy link
Member

Choose a reason for hiding this comment

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

It looks like we may need to update the stability here to experimental as well?

* Returns the details about a search application
* @rest_spec_name search_application.get
* @since 8.8.0
* @stability beta
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to update these @stability tags too

Copy link
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

This looks good to me!

@kderusso We'll eventually need to update the stability tags, but I was thinking we could get this PR merged and then I'll manually resync the rest-api-spec stubs before fixing the stabilities. Otherwise we need to resync, rebase, and all that, might be easier this way?

@carlosdelest
Copy link
Member Author

@kderusso , as @sethmlarson said, this can be done in a separate PR with the syncs already done. I'm merging this PR and we'll take care of that in a separate one 👍

@carlosdelest carlosdelest merged commit 46f9022 into main Apr 19, 2023
@carlosdelest carlosdelest deleted the carlosdelest/search-application-api branch April 19, 2023 18:28
swallez pushed a commit that referenced this pull request Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants