Add UI components and polling for zip downloads #906
Conversation
mcannalte
force-pushed
the
feat/download
branch
from
3df9c66 to
b5d4883
Compare
mcannalte
changed the title
mcannalte
force-pushed
the
feat/download
branch
from
b5d4883 to
de85ebb
Compare
There was a problem hiding this comment.
Can you deploy this feature somewhere so ppl can test?
Also would suggest to move the large amount of text in your current PR comment to descriptions and for those individual sections (New features, Improvements, Bug fixes etc), you should only keep a few brief bullet points as summerization. So it can be picked up by the gen3-release-helper script better when releasing
Also you should mention this feature depends on the 2 other PRs as deployment changes
mcannalte
force-pushed
the
feat/download
branch
from
d3db197 to
1cfcda9
Compare
There was a problem hiding this comment.
since you have added more config field into portal config, can you also update https://github.com/uc-cdis/data-portal/blob/master/docs/portal_config.md
src/Discovery/DiscoveryActionBar.tsx
Outdated
| }, | ||
| ).catch(handleJobError); | ||
| } else { | ||
| setTimeout(pollForJobStatusUpdate, 5000); |
There was a problem hiding this comment.
This will cause the component stuck in an infinite loop if uid is not returned from https://github.com/uc-cdis/data-portal/pull/906/files#diff-1ac68dcc0067af6b35b142c058129318cc50ee20bfa9195afdb3d99f150d9d15R81
You can reproduce it now in qa-heal, since the mock auth is turned on, you will be login by default as a test account, which doesn't have sower access. The job dispatch request will return as 403 error, and no uid is returned, but this section of code will repeatly checking with uid=undefined
Better to handle fetched response with HTTP status code, rather than plain text
There was a problem hiding this comment.
True. I will add some checks for HTTP status code for cases like this. For the /status calls, however, the status code from Sower is 200 regardless of job state, so I will have to keep some plain text checks in there
There was a problem hiding this comment.
make sense! probably only the first call to /job/dispatch needs a status code check
Jira Ticket: https://ctds-planx.atlassian.net/browse/HP-365
New Features
Deployment Changes
batch-exportsower job: Batch export sower job sower-jobs#29 in the environment's manifest. This job also has its own setup in cloud automation: Feat/batch export cloud-automation#1708gitops.json:discoveryconfig.features.exportToWorkspace.[enableDownloadZip (bool) downloadZipButtonText (string)]