8525 ingest optional skip #8532

lubitchv · 2022-03-24T17:51:21Z

What this PR does / why we need it: This PR adds optional parameter to native add file to dataset API. The parameter is tabIngest. By default it is true. If tabIngest is false then api call skips tabular ingest.

Which issue(s) this PR closes:

Closes #8525

Suggestions on how to test this:
Test testAddFileToDatasetTabIngest was added in DatasetsIT.
One can also test this api call using curl:
curl -H X-Dataverse-key:$API_TOKEN -X POST -F "file=@test.sav" -F 'jsonData={"description":"My description.","directoryLabel":"data/subdir1","categories":["Data"], "restrict":"false", "tabIngest":"false"}' http://localhost:8080/api/datasets/$DATASET_ID/add

Update from IQSS

Update

coveralls · 2022-03-24T18:00:52Z

Coverage increased (+0.009%) to 18.897% when pulling fe9e7e6 on lubitchv:8525-injest-optional-skip into c5d1df2 on IQSS:develop.

pdurbin

Overall, this pull request looks great. It's API-only but sets the stage for a future GUI to allow users to skip ingest (#2199).

I do have a few small items to look at. Please see the review. Thanks!

doc/sphinx-guides/source/api/native-api.rst

src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java

Co-authored-by: Philip Durbin <philipdurbin@gmail.com>

…erse into 8525-injest-optional-skip

qqmyers · 2022-03-28T23:02:50Z

FWIW: The current failure is just #8533 - not related to your PR.

pdurbin · 2022-03-29T14:26:34Z

src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java

+        String pathToFile = "src/test/resources/sav/dct.sav";
+        String jsonAsString = "{\"description\":\"My description.\",\"directoryLabel\":\"data/subdir1\",\"categories\":[\"Data\"], \"restrict\":\"false\", \"tabIngest\":\"false\"}";
+        Response r = UtilIT.uploadFileViaNative(datasetIdInt.toString(), pathToFile, jsonAsString, apiToken);
+        logger.info(r.prettyPrint());
+        assertEquals(200, r.getStatusCode());
+
+        pathToFile = "src/test/resources/sav/frequency-test.sav";
+        jsonAsString = "{\"description\":\"My description.\",\"directoryLabel\":\"data/subdir1\",\"categories\":[\"Data\"], \"restrict\":\"false\"  }";
+        Response rTabIngest = UtilIT.uploadFileViaNative(datasetIdInt.toString(), pathToFile, jsonAsString, apiToken);
+        logger.info(rTabIngest.prettyPrint());
+        assertEquals(200, rTabIngest.getStatusCode());


It might be nice to assert somehow if the file was ingested or not. I'm not sure about the easiest way. Maybe UtilIT.downloadTabularFile? Or check if there's a UNF?

Checking the file name may be enough (making sure it's still ".sav", and not ".tab").

But yes, I agree, we do need to confirm that the ingest was indeed skipped.
Also, the 2nd file upload in this method - where ingest is NOT being skipped - it may not be necessary, since we are already testing actual ingest elsewhere (in FilesIT.java?).

Ok, I renamed the test and moved it to FilesIT. I also added the ingest check by checking label of file metadata.

src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java

pdurbin

I haven't run the code but I'm clicking Approve. Looks good. Thanks, @lubitchv!

However, I'm going to keep this PR in Review in case @landreev wants to take a look.

@lubitchv I did leave you a comment about adding extra assertions that the .sav file was or wasn't ingested, if you feel like it.

landreev · 2022-03-29T18:53:29Z

doc/sphinx-guides/source/api/native-api.rst

@@ -1301,6 +1301,7 @@ When adding a file to a dataset, you can optionally specify the following:
 - A description of the file.
 - The "File Path" of the file, indicating which folder the file should be uploaded to within the dataset.
 - Whether or not the file is restricted.
+- Whether or not the file skips tabular ingest.


This should probably say that it defaults to "true" (?).

I added the phrase that if tabIngest is not specified then it defaults to true.

landreev · 2022-03-29T19:21:59Z

src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java

@@ -2583,5 +2583,48 @@ public void testFilesUnchangedAfterDatasetMetadataUpdate() throws IOException {
                .body("data.latestVersion.files[0].directoryLabel", equalTo("code"));

    }
+
+    @Test
+    public void testAddFileToDatasetTabIngest() throws IOException, InterruptedException {


It would make sense to rename this test method, to make it clear that it is specifically testing skipping ingest; to differentiate it from all the other ingest tests in FilesIT etc.
(Thinking about it, maybe this method needs to be moved to FilesIT.java too?)

I renamed it to testAddFileToDatasetSkipTabIngest and moved it to FilesIT.

landreev

Looks great, thank you!

landreev · 2022-03-31T20:13:12Z

src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java

+        Response r = UtilIT.uploadFileViaNative(datasetIdInt.toString(), pathToFile, jsonAsString, apiToken);
+        logger.info(r.prettyPrint());
+        assertEquals(200, r.getStatusCode());
+


Sorry to keep bugging you but, since this hasn't been merged yet, could you please insert another sleepForLock statement here? (Otherwise, if the ingest somehow ends up happening, despite the tabIngest:false above, we will likely fail to detect that - because the file name will be checked before it gets updated!)
It should be the same entry as in line 1820:

assertTrue("Failed test if Ingest Lock exceeds max duration " + pathToFile, UtilIT.sleepForLock(datasetIdInt, "Ingest", apiToken, UtilIT.MAXIMUM_INGEST_LOCK_DURATION));

Thanks. (I should've thought about this sooner of course).

Added sleep for lock.

Great, thanks.

kcondon · 2022-04-08T20:12:07Z

@lubitchv would you mind refreshing from develop branch since we've had a version change?

Update

lubitchv · 2022-04-11T13:53:10Z

Updated

kcondon · 2022-04-11T18:15:40Z

@lubitchv Can you take a look at this test that is failing with your branch? https://github.com/lubitchv/dataverse/blob/8525-injest-optional-skip/src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java#L2243

pdurbin · 2022-04-11T18:24:28Z

@lubitchv just to give a little more detail on the fail test above:

org.xml.sax.SAXParseException:
Unexpected EOF in prolog
at [row,col {unknown-source}]: [1,0]
at edu.harvard.iq.dataverse.api.DatasetsIT.testRestrictFileExportDdi(DatasetsIT.java:2243)
Caused by: com.ctc.wstx.exc.WstxEOFException:
Unexpected EOF in prolog
at [row,col {unknown-source}]: [1,0]
at edu.harvard.iq.dataverse.api.DatasetsIT.testRestrictFileExportDdi(DatasetsIT.java:2243)

Something to do with exporting the DDI format, it seems.

lubitchv · 2022-04-11T21:23:37Z

I run DatasetsIT.testRestrictFileExportDdi on my machine. It was fine. Maybe it is database issue?

pdurbin · 2022-04-12T14:04:22Z

@lubitchv you're right. DatasetsIT runs fine for me on my laptop as of ecc9ffc.

I just pushed a couple small doc changes to your branch to force Jenkins to run again. Let's check back later today to see if the tests pass.

qqmyers · 2022-04-12T16:28:51Z

FWIW: My suspicion is that the test fail here was related to gdcc/dataverse-ansible#231. Don turned off having the tests run with the languages and metadata languages features set to en or de-DE as of last night.

lubitchv and others added 10 commits March 18, 2022 12:11

Merge pull request #59 from IQSS/develop

8b9e706

Update from IQSS

Merge pull request #60 from IQSS/develop

b9b2efe

Update

tab ingest skip option

e504bfc

tabIngest optional param

0943258

small fixes

d0c70e7

docs

46d07ce

UtilIT back

3f92d83

boolean

5cbe6a7

src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java

f0123ec

style

1a040a6

style

8457911

style

d5090b7

poikilotherm mentioned this pull request Mar 25, 2022

8531 refactor validators #8534

Merged

pdurbin self-assigned this Mar 28, 2022

pdurbin reviewed Mar 28, 2022

View reviewed changes

doc/sphinx-guides/source/api/native-api.rst Outdated Show resolved Hide resolved

src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java Outdated Show resolved Hide resolved

src/test/java/edu/harvard/iq/dataverse/api/DatasetsIT.java Show resolved Hide resolved

pdurbin assigned lubitchv Mar 28, 2022

lubitchv and others added 6 commits March 28, 2022 15:29

json null replace

2da5d2d

Remove hardcoded sleep

eea514e

Update doc/sphinx-guides/source/api/native-api.rst

3170ef2

Co-authored-by: Philip Durbin <philipdurbin@gmail.com>

optionalFileParams canbe null

9b04c9c

Merge branch '8525-injest-optional-skip' of github.com:lubitchv/datav…

3589b4c

…erse into 8525-injest-optional-skip

json

4fc0a35

lubitchv requested a review from pdurbin March 29, 2022 13:46

pdurbin reviewed Mar 29, 2022

View reviewed changes

pdurbin requested a review from landreev March 29, 2022 14:31

pdurbin approved these changes Mar 29, 2022

View reviewed changes

landreev reviewed Mar 29, 2022

View reviewed changes

landreev self-assigned this Mar 29, 2022

landreev reviewed Mar 29, 2022

View reviewed changes

lubitchv added 2 commits March 29, 2022 16:13

test and doc

2163fdc

assert

3cf56e8

pdurbin changed the title ~~8525 injest optional skip~~ 8525 ingest optional skip Mar 29, 2022

landreev approved these changes Mar 29, 2022

View reviewed changes

landreev removed their assignment Mar 29, 2022

pdurbin unassigned pdurbin and lubitchv Mar 30, 2022

landreev reviewed Mar 31, 2022

View reviewed changes

add waitForLock

fe9e7e6

Merge pull request #63 from IQSS/develop

ecc9ffc

Update

kcondon self-assigned this Apr 11, 2022

pdurbin added 2 commits April 12, 2022 10:00

link to tabular ingest docs IQSS#8525

47c1413

add release note IQSS#8525

66b774b

kcondon merged commit 9094b94 into IQSS:develop Apr 12, 2022

pdurbin added this to the 5.11 milestone Apr 12, 2022

pdurbin mentioned this pull request Oct 1, 2022

RData ingest #6985

Closed

pdurbin mentioned this pull request Oct 13, 2022

Support uploading of archives (ZIP, other). #8029

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8525 ingest optional skip #8532

8525 ingest optional skip #8532

lubitchv commented Mar 24, 2022

coveralls commented Mar 24, 2022 •

edited

Loading

pdurbin left a comment

qqmyers commented Mar 28, 2022

pdurbin Mar 29, 2022

landreev Mar 29, 2022

landreev Mar 29, 2022

lubitchv Mar 29, 2022

pdurbin left a comment

landreev Mar 29, 2022

lubitchv Mar 29, 2022

landreev Mar 29, 2022

lubitchv Mar 29, 2022

landreev left a comment

landreev Mar 31, 2022

lubitchv Mar 31, 2022

landreev Mar 31, 2022

kcondon commented Apr 8, 2022

lubitchv commented Apr 11, 2022

kcondon commented Apr 11, 2022

pdurbin commented Apr 11, 2022

lubitchv commented Apr 11, 2022

pdurbin commented Apr 12, 2022

qqmyers commented Apr 12, 2022

8525 ingest optional skip #8532

8525 ingest optional skip #8532

Conversation

lubitchv commented Mar 24, 2022

coveralls commented Mar 24, 2022 • edited Loading

pdurbin left a comment

Choose a reason for hiding this comment

qqmyers commented Mar 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pdurbin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

landreev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kcondon commented Apr 8, 2022

lubitchv commented Apr 11, 2022

kcondon commented Apr 11, 2022

pdurbin commented Apr 11, 2022

lubitchv commented Apr 11, 2022

pdurbin commented Apr 12, 2022

qqmyers commented Apr 12, 2022

coveralls commented Mar 24, 2022 •

edited

Loading