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

feat(api-v2): Add support for text file upload (DSP-44) #1664

Merged
merged 16 commits into from Sep 29, 2020

Conversation

benjamingeer
Copy link

@benjamingeer benjamingeer commented Jun 29, 2020

https://dasch.myjetbrains.com/youtrack/issue/DSP-44

@benjamingeer benjamingeer self-assigned this Jun 30, 2020
@benjamingeer benjamingeer marked this pull request as draft June 30, 2020 06:47
@subotic subotic changed the title feat(api-v2): Add support for text file upload feat(api-v2): Add support for text file upload (DSP-44) Sep 23, 2020
@benjamingeer benjamingeer marked this pull request as ready for review September 28, 2020 12:42
@benjamingeer
Copy link
Author

Skipping ClientApiRouteE2ESpec because it keeps failing with this error, despite everything we've tried:

Failed to get test data: {"knora-api:error":"akka.pattern.AskTimeoutException: Ask timed out on
[Actor[akka://E2ETest/user/applicationManager#-1049986081]] after [5000 ms]. Message of type
[org.knora.webapi.messages.admin.responder.usersmessages.UserGetADM] was sent by
[Actor[akka://E2ETest/user/applicationManager#-1049986081]].

@subotic
Copy link
Collaborator

subotic commented Sep 28, 2020

@benjamingeer @SepidehAlassi could you please make sure to fix this test either in this PR or as the first next thing? If you decide to ignore it in this PR, please open an issue on YouTrack. This test is also failing on develop.

@benjamingeer
Copy link
Author

@benjamingeer @SepidehAlassi could you please make sure to fix this test either in this PR or as the first next thing? If you decide to ignore it in this PR, please open an issue on YouTrack. This test is also failing on develop.

We're planning to redesign it in the next sprint, using your idea of collecting the test data as a side effect of running the E2E tests.

@subotic
Copy link
Collaborator

subotic commented Sep 29, 2020

cool, great 👍

Copy link
Contributor

@SepidehAlassi SepidehAlassi left a comment

Choose a reason for hiding this comment

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

Thanks for this, please see my comment regarding work around for DSP-711

test_data/test_route/files/spam.csv Outdated Show resolved Hide resolved
sipiResponseStr <- doSipiRequest(sipiRequest)
sipiResponse: SipiKnoraJsonResponse = sipiResponseStr.parseJson.convertTo[SipiKnoraJsonResponse]

// Workaround for https://dasch.myjetbrains.com/youtrack/issue/DSP-711
Copy link
Contributor

Choose a reason for hiding this comment

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

@benjamingeer it seems like Lukas has already fixed the issue of DSP-711, see his response should this workaround be removed in this PR or later?

Copy link
Contributor

@SepidehAlassi SepidehAlassi left a comment

Choose a reason for hiding this comment

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

@benjamingeer thanks for your work. As agreed, please make a new PR with the new version of SIPI.

@subotic
Copy link
Collaborator

subotic commented Sep 29, 2020

PR for updating Sipi: #1721

@benjamingeer
Copy link
Author

@SepidehAlassi Thanks for reviewing!

@benjamingeer benjamingeer added API/V2 enhancement improve existing code or new feature Sipi labels Sep 29, 2020
@benjamingeer benjamingeer merged commit a88d20d into develop Sep 29, 2020
@benjamingeer benjamingeer deleted the wip/DSP-44-text-files branch September 29, 2020 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API/V2 enhancement improve existing code or new feature Sipi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants