WIP: Feat / 48 Configurable Jbrowse Wrapper #197
Conversation
| const arrangerFetcher = createArrangerFetcher({}); | ||
|
|
||
| const fileQuery = `file { |
There was a problem hiding this comment.
Could be its own json file
| } | ||
| } | ||
| } | ||
| ${dataQuery} |
There was a problem hiding this comment.
Use interchangeable query bodies
There was a problem hiding this comment.
great start! not depending on the hard coded query, the root of the issue.
There was a problem hiding this comment.
OK great, wasn't sure if we were simply 'swapping queries' or if we were potentially retrieving data from a source other than Arranger
| const isDefaultDataModel = !NEXT_PUBLIC_JBROWSE_DATA_MODEL ? true : false; | ||
| // TODO: Handle Alternate Data Model | ||
| const nodes = isDefaultDataModel ? data.file?.hits?.edges : []; |
There was a problem hiding this comment.
I like what you're doing here, and the syntax can be simplified further.
right now it reads as isDefaultMode is true if the condition (without the env var being set) is true, and false if the condition is false. without the "true if true, false if false" redundancy, you could just do const isDefaultDataModel = !NEXT_PUBLIC_JBROWSE_DATA_MODEL;
That said, when the data model is not the default one, then you're not using the data at all, right?
the code in line 155+ will be filtering over an empty array every time the data model is not the default one.
Thinking there's more to be done in this part of the code to account for that model change.
There was a problem hiding this comment.
Yes that's why I added the TODO at 151. For now I don't have an example definition for alternate data models, so there's no code to write. Will simplify the conditional.
There was a problem hiding this comment.
And what I mean here is the params in 158-162, etc. will change if the data model is different.
I have a feeling this whole thing will have to be refactored into using env variables to determine the "json path" for file size, and for file type, and so on.
There was a problem hiding this comment.
Exactly -- this is why I kept the empty array for now and have not updated lines 151+ onward. Data model is ambiguous so we need something to help navigate various data models. JSON Path seems like it might be the perfect tool.
demariadaniel
changed the title
| const nodes = isDefaultDataModel | ||
| ? data.file?.hits?.edges | ||
| : jsonpath | ||
| .query(data, '$..edges')[0] |
There was a problem hiding this comment.
Given Jbrowse compatibility filter and file mapping requires specific properties, you kind of have to know what the input query is going to be.
Here I am querying edges[0] because it allows me to get all the nodes and then map them. I might be able to simplify this a bit by querying nodes, but you still need that identifier to know the name of your objects.
If your query doesn't have edges (or nodes), it won't work. The query can't be completely ambiguous.
We might be able to do a .find() to get an array of all objects with the given fields we need as an alternate solution
cc: @justincorrigible
| } | ||
| } | ||
| } | ||
| // TODO: Add Filters back in ($filters:JSON) |
There was a problem hiding this comment.
I'm actually not sure how variables will work in this format. Adding $filters to the env variable causes gql validation errors; adding it to the jbrowseinput query string causes a filters is unused error.
cc: @justincorrigible
| score: '', | ||
| sort: [{ fieldName: 'analysis_id', order: 'asc' }], | ||
| sqon: null, | ||
| }, |
There was a problem hiding this comment.
This will not be appropriate for all queries as well so this is another piece to revisit
| node: { | ||
| data_type: string; | ||
| object_id: string; | ||
| name: string; | ||
| size: number; | ||
| fileType: string; | ||
| file_access: string; | ||
| }; |
There was a problem hiding this comment.
the names in this properties cannot be guaranteed in the input model
e.g. fileType, vs file_type, vs file.type
| @@ -104,6 +104,27 @@ export const jbrowseFileMetadataQuery = ` | |||
| } | |||
| `; | |||
|
|
|||
| export const fileQuery = `file { | |||
There was a problem hiding this comment.
thinking it would be useful to call this defaultArrangerFileQuery or something like that to make it more declarative
demariadaniel
changed the title
https://app.zenhub.com/workspaces/overture-stack-5d2e058ff67cc800011fee6b/issues/gh/overture-stack/dms-jbrowse-components/48
filequery handling and dynamic env queries