Jbrowse2 service

[steve-fischer-200]

No description provided.

[ryanrdoherty]

If you use Javadoc here (two **s), our API may eventually pick it up. Doesn't now because we are using hard-coded RAML. :(

[ryanrdoherty]

Maybe better to not merge the streams and instead collect error output into a String and check if non-empty. Don't know the characteristics of the subprocess. I see you're not checking return value either, so how do you know if it bombed? Would be nice to log the subprocess output in the case of runtime error- Cristina will definitely ask for this later.

[ryanrdoherty]

Need to wait for the process to complete. Also I think the output collection could be simpler. Something like this:

    String stringFromCommand(List<String> command) throws IOException {
      try {
        Process p = processFromCommand(command);

        ByteArrayOutputStream stringBuffer = new ByteArrayOutputStream();
        p.getInputStream().transferTo(stringBuffer);
        String errors = stringBuffer.toString();

        stringBuffer.reset();
        p.getInputStream().transferTo(stringBuffer);

        if (p.waitFor() != 0) {
          throw new RuntimeException("Subprocess from [" + String.join(" ", command) + "] returned non-zero.  Errors:\n" + errors);
        }

        return stringBuffer.toString();
      }
      catch (InterruptedException e) {
        throw new RuntimeException("Subprocess from [" + String.join(" ", command) + "] was interrupted befor it could complete.");
      }
    }

[ryanrdoherty]

You forgot to add subAdapter anywhere. A nice feature of org.json is the ability to chain the calls together, so you can avoid misses like this. It also mirrors the structure of the resulting JSON, increasing readability. See below:

    JSONObject createBigwigTrackJson(String vdiId, String vdiName, String fileName, String organismAbbrev, String userDatasetsFilePath) {
      return new JSONObject()
          .put("assemblyNames", new JSONArray().put(organismAbbrev))
          .put("trackId", vdiId)
          .put("name", vdiName)
          .put("displays", new JSONArray()
              .put(new JSONObject()
                  .put("displayId", "wiggle_ApiCommonModel::Model::JBrowseTrackConfig::MultiBigWigTrackConfig::XY=HASH(0x2249320)")
                  .put("maxScore", 1)
                  .put("maxScore", 1000)
                  .put("defaultRendering", "multirowxy")
                  .put("type", "MultiLinearWiggleDisplay")
                  .put("scaleType", "log")))
          .put("adapter", new JSONObject()
              .put("subadapters", new JSONArray()
                  .put(new JSONObject()
                      .put("color1", "grey")
                      .put("name", fileName)
                      .put("type", "BigWigAdapter")
                      .put("bigWigLocation", new JSONObject()
                          .put("locationType", "UriLocation")
                          .put("uri", String.join("/", userDatasetsFilePath, vdiId, fileName))))));
    }

[ryanrdoherty]

A little worried about the size of this. toString() means doubling the size of the completed file before returning. Wondering if we should be using Jackson to stream as Ellie suggested. Though that can probably wait until we know this is giving us what we want.

[ryanrdoherty]

Does this work? Seems like you would want table refs in these columns. Also, is sharing covered in AvailableUserDatasets? is_public is our community datasets indicator?

[ryanrdoherty]

So, we can't return to the web client the full path to these files. We're returning relative URLs. Right now there's a script in service-jbrowse2 that does this transform using sed (depends on regex matching per line), but we should probably use jq so John can send the JSON in a single line. He mentioned this in one of our meetings. But wonder if we could be doing that here? We can at least set the correct value up front for user datasets.

[ryanrdoherty]

Don't need to advertise this. :)

[ryanrdoherty]

Hmm.. maybe this is handling the filepath -> relative URL comment I made earlier. Should maybe add comments to talk about what is returned for the "location" of this track?