Alter where API stores/finds adhoc questions and answers

[corinaminer]

Improved alternative to #678. No new API calls, but the existing API calls have been modified to anticipate this storage setup:

myContainer
    |- analyses (contents unchanged)
    |- questions
        |- myAdhocQuestion
            |- question.json
    |- testrigs
        |- myTestrig
            |- answers
                |- myAdhocQuestion
                    |- myTestrigEnv
                        |- standard
                            |- answer.json
                        |- differential
                            |- myDeltaTestrig
                                |- myDeltaEnv
                                    |- answer.json

Testrig directories are no longer expected to contain a folder called questions, so old testrigs will lose track of their existing adhoc questions when this is merged.

---

This change is 

[dhalperi]

Reviewed 5 of 5 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

projects/batfish/src/main/java/org/batfish/main/Batfish.java, line 4480 at r1 (raw file):

      CommonUtil.writeFile(jsonPath, answerString);
    }
    Path questionPath = _settings.getQuestionPath();

what is settings.getQuestionPath and is it actually used for anything?

---

projects/coordinator/src/main/java/org/batfish/coordinator/WorkMgrService.java, line 361 at r1 (raw file):

      @FormDataParam(CoordConsts.SVC_KEY_CONTAINER_NAME) String containerName,
      @FormDataParam(CoordConsts.SVC_KEY_QUESTION_NAME) String questionName,
      @FormDataParam(CoordConsts.SVC_KEY_TESTRIG_NAME) String testrigName) {

delete argument?

(I believe that the server will accept and ignore additional form parameters, but you should confirm this.)

---

projects/coordinator/src/test/java/org/batfish/coordinator/WorkMgrTest.java, line 112 at r1 (raw file):

  @Test
  public void listQuestionWithNonExistTestrig() {

this test has to go, I think.

---

Comments from Reviewable

[corinaminer]

Review status: 3 of 5 files reviewed at latest revision, 3 unresolved discussions, some commit checks failed.

---

projects/batfish/src/main/java/org/batfish/main/Batfish.java, line 4480 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

what is settings.getQuestionPath and is it actually used for anything?

My understanding is that for tasks that use work items, it's easier to stash relevant parameters like question, container, testrig, etc. into settings than to carry them through function parameters, because the execution path for those tasks isn't linear. Then using settings.getQuestionPath makes sense because settings already knows which container and question you're after.

---

projects/coordinator/src/main/java/org/batfish/coordinator/WorkMgrService.java, line 361 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

delete argument?

(I believe that the server will accept and ignore additional form parameters, but you should confirm this.)

Done.

---

projects/coordinator/src/test/java/org/batfish/coordinator/WorkMgrTest.java, line 112 at r1 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

this test has to go, I think.

Done, and fixed other tests that were looking for questions inside testrig directories.

---

Comments from Reviewable

[dhalperi]

Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.

---

a discussion (no related file):
Tests are failing because of null pointer exceptions somewhere.

---

Comments from Reviewable

[dhalperi]

projects/batfish/src/main/java/org/batfish/main/Batfish.java, line 4468 at r3 (raw file):

        answerDir = answerDir.resolve(Paths.get(BfConsts.RELPATH_STANDARD_DIR));
      }
    } else if (_settings.getAnalysisName() != null) {

still failing. My bet is that it's writing the log for the overall analysis as well, and then analysisName is present but not questionPath. Then, based on that guess, would be that you should check both.

---

Comments from Reviewable

[dhalperi]

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.

---

Comments from Reviewable

[dhalperi]

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[corinaminer]

Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks pending.

---

projects/batfish/src/main/java/org/batfish/main/Batfish.java, line 4468 at r3 (raw file):

Previously, dhalperi (Dan Halperin) wrote…

still failing. My bet is that it's writing the log for the overall analysis as well, and then analysisName is present but not questionPath. Then, based on that guess, would be that you should check both.

Done, and it now passes the test it was failing (run-analysis in ui-focused).

---

Comments from Reviewable