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

Uses POST instead of GET for CloudSearch Domain search operation #976

Merged
merged 3 commits into from
Apr 30, 2016

Conversation

LiuJoyceC
Copy link
Contributor

Converts search requests to CloudSearch Domain to use POST instead of GET to lift restriction on query length. Resolves #614

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 88.579% when pulling 12c31bb on LiuJoyceC:cloudSearchDomainGetToPost into a1027ec on aws:master.

req = new AWS.CloudSearchDomain({endpoint: 'host.domain.com'}).search({query: 'foo', cursor: 'initial', queryOptions: '{}'}).build()
req = new AWS.CloudSearchDomain({endpoint: 'host.domain.com'})
.search({query: 'foo', cursor: 'initial', queryOptions: '{}'})
.removeListener('build', AWS.CloudSearchDomain.prototype.convertGetToPost)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this listener being removed in the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test is testing whether the query string is alphabetically sorted by its field names. The listener that I'm removing here previously did not exist, as it was added as part of this PR, so I'm not removing something that used to be there. Since the listener I added in this PR changes the GET request to a POST, there is no longer a query string, and the test cannot check if it's alphabetically sorted. Therefore, I removed the listener so that just for the purpose of this test, the request is still a GET and has a query string. The other two options I have if I don't remove this listener is to switch the test to use a different operation (I could use suggest instead of search) or to remove this test altogether.

@LiuJoyceC LiuJoyceC force-pushed the cloudSearchDomainGetToPost branch from 12c31bb to be1052c Compare April 29, 2016 23:41
@coveralls
Copy link

coveralls commented Apr 29, 2016

Coverage Status

Coverage increased (+0.03%) to 88.579% when pulling be1052c on LiuJoyceC:cloudSearchDomainGetToPost into 0bfea05 on aws:master.

@chrisradek
Copy link
Contributor

:shipit:

@LiuJoyceC LiuJoyceC merged commit 7cc0eee into aws:master Apr 30, 2016
@LiuJoyceC LiuJoyceC deleted the cloudSearchDomainGetToPost branch April 30, 2016 00:05
LiuJoyceC added a commit that referenced this pull request May 3, 2016
@lock
Copy link

lock bot commented Sep 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle CloudSearch Query Limit
3 participants