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

8073 guestbook response api #8084

Merged
merged 11 commits into from
Sep 16, 2021
Merged

8073 guestbook response api #8084

merged 11 commits into from
Sep 16, 2021

Conversation

sekmiller
Copy link
Contributor

@sekmiller sekmiller commented Sep 1, 2021

What this PR does / why we need it: The Retrieve Guestbook Responses api was failing for DVs with large numbers of responses. Creating a file for the responses was stepping on the streaming of results.

Which issue(s) this PR closes:

Closes #8073 Retrieve Guestbook Responses fails for some collections

Special notes for your reviewer:

Suggestions on how to test this:

Edit (L.A.): The guestbook for the covid-rt dataverse (the one with the regularly updated datasets/hundreds of versions) started this issue (it could not be downloaded because of the prod. timeout). So it should be a good test on a prod. db copy (/api/dataverses/covid-rt/guestbookResponses). Before this PR, you would start receiving output after about 2 min 40 sec. Should be ~40 sec now. Still not instant, but an improvement.
Also, note the simplified curl command line in the guide.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?: Included

Additional documentation: updated doc for end user.

Copy link
Contributor

@landreev landreev left a comment

Choose a reason for hiding this comment

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

@sekmiller In its current state, did you have a chance to test this code with an actual giant guestbook? (on a copy of the prod. db, for example)

@landreev
Copy link
Contributor

landreev commented Sep 2, 2021

@sekmiller The API in its current form still doesn't stream the output it generates.
I'm going to move this back into "in progress", and switch to the slack thread.

@landreev landreev removed their assignment Sep 2, 2021
@coveralls
Copy link

coveralls commented Sep 3, 2021

Coverage Status

Coverage increased (+0.001%) to 19.154% when pulling e19bbc0 on 8073-guestbook-response-api into 8127559 on develop.

@sekmiller sekmiller removed their assignment Sep 9, 2021
Copy link
Contributor

@landreev landreev left a comment

Choose a reason for hiding this comment

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

It is definitely streaming now - as in, it starts sending the output back to the client as soon as it becomes available.
Unfortunately, "as soon as becomes available" may still not be soon enough for that covid-rt dataverse guestbook (from the original report that started this issue) as to start downloading before the prod. timeout. This is because for that particular dataverse it is the database query itself that appears to be taking more than 2 minutes. I am assuming this is because of that monstrous mess of versions in every dataset there. (This is that dataverse with datasets that are being continuously updated, resulting in versions in the hundreds). I should have realized this sooner, when I tested the previous iteration of the PR.

Before moving this into QA, I'm trying to quickly determine if there is a simple database optimization approach that may make this API much more responsive. (our approach of generating the result list with just one db query may be counterproductive there).

Copy link
Contributor

@landreev landreev left a comment

Choose a reason for hiding this comment

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

Moving into QA.

@kcondon kcondon assigned kcondon and unassigned landreev Sep 14, 2021
@kcondon kcondon merged commit 35ec16f into develop Sep 16, 2021
@kcondon kcondon deleted the 8073-guestbook-response-api branch September 16, 2021 14:56
@djbrooke djbrooke added this to the 5.7 milestone Sep 16, 2021
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.

"Retrieve Guestbook Responses for a Dataverse Collection" fails for some collections
5 participants