EN-31530: add support for additional layers (with query, context, and copy) #48
Conversation
this parameter is a list of uids that tells the export service which of the views the soql query should be applied to. This is to support the exporting of viz can maps, where a user may have saved a query that applies only to the base layer of the map
…ll layers; backwards-compatibility
catstavi
changed the title
| val copy = req.queryParameter("copy").getOrElse("published") | ||
| val context = req.queryParameter("context").getOrElse("{}") | ||
| layers: Seq[LayerWithQuery]): Either[HttpResponse, Seq[Response with Closeable]] = { | ||
| val defaultSoql = s"select * limit ${Int.MaxValue}" |
There was a problem hiding this comment.
The limit parameter is required (and explicitly as big as we can make it) to ensure that we don't fall back to a default limit?
There was a problem hiding this comment.
Not my addition, but yep, that is my understanding, is that if you don't specify a limit the NBE defaults to 10000 or something.
| .addHeader(ReqIdHeader -> req.requestId) | ||
| .addHeader("X-Socrata-Lens-Uid" -> fbf) | ||
| .addHeader("X-Socrata-Lens-Uid" -> layer.uid) |
There was a problem hiding this comment.
Woah. First time seeing this header. I realize you haven't introduced a change here, but can you give me a little background on this?
There was a problem hiding this comment.
gosh. I was going to say, no idea, but I was poking in git commit history and it looks like I did add this. I have no explanation other than I probably saw it was being sent by core to geo-export and soda-fountain, and decided to pass it along.
Looks like it was at one point related to views/rows loaded metrics?
| @@ -122,13 +127,79 @@ class ExportService(sodaClient: UnmanagedCuratedServiceClient) extends SimpleRes | |||
| } | |||
| } | |||
|
|
|||
| private def parseQueryBody(req: HttpRequest, baseUids: Seq[String]): Either[HttpResponse, Seq[LayerWithQuery]] = { | |||
There was a problem hiding this comment.
This method is information-dense and has me wondering if it could be refined for readability...
There was a problem hiding this comment.
This might even be more information dense, but I tend to find for comprehensions more readable:
private def parseQueryBody(req: HttpRequest, baseUids: Seq[String]): Either[HttpResponse, Seq[LayerWithQuery]] = {
for {
reader <- req.reader.left.map(_ =>
BadRequest ~> Json(JsonUtil.renderJson(Map(errorKey -> "Invalid or unparsable Content-Type or MimeType")))
).right
queryBody <- JsonUtil.readJson[QueryBody](reader).left.map(_ =>
BadRequest ~> Json(JsonUtil.renderJson(Map(errorKey -> s"""Could not parse the QUERY body""")))
).right
layersWithQueries <- JsonUtil.parseJson[Seq[LayerWithQuery]](queryBody.layersWithQueries).left.map(_ =>
BadRequest ~> Json(JsonUtil.renderJson(Map(errorKey -> s"""Could not parse the layersWithQuerys parameter""")))
).right
} yield {
baseUids.map(fxf => LayerWithQuery(fxf, queryBody.query, queryBody.context, queryBody.copy)) ++ layersWithQueries
}
}There was a problem hiding this comment.
I ended up modifying how the query body is passed so that LayersWithQueries gets passed as regular json and not a string, and thus doesn't need to be parsed out separately! That leaves it with just two layers of Right/Left matching, which I find easier to read in this format than a for comprehension, if that's okay?
|
@rlvoyer this is ready for re-review if you want to check on the changes! |
Geo-export previously supported receiving a list of nbe resource_names, and optional query, context, and copy information. In order to support the export of viz can maps, we need to be able to support each layer having its own independent query and context information. For example, one layer of the viz can map may be a derived view that represents a filter of the true NBE dataset.
This change teaches geo-export to accept a new parameter, "layersWithQueries", which is a list of "layerWithQuery" objects, each of which has a resource name, a query, a context, and a copy indicator. It composes this parameter with the other string of layer uids and base-level query/context/copy info it receives, to make the ultimate request to soda fountain. The new parameter is optional, meaning that export requests of the previous shape will continue to work (allowing us to deploy geo-export before core)
Additionally, this changes adds support for the "QUERY" http method, in case the layersWithQueries parameters causes the GET request to reach an invalid length, core will instead switch over to using "QUERY", and include the parameters in the body of the request.