I had some suggestions when looking through the GraphQL API as it exists now.
My main comment is that our API still feels a little "REST-like" due to the fact we are using DRF strings in our requests. Instead, I propose we move away from DRF strings and instead model them in GraphQL's type system.
Querying device properties
Instead of relying on the DRF string to denote what specific property of a device the request is asking for, we should instead represent a device as a GraphQL object.
This could look something like this:
type Device{
# metadata of our device
description: String,
index: Int,
alarmListName: String
# ... whatever other metadata we support??
# all raw values are a byte array represented as an array of graphQL ints
analogReadingRaw: [Int!],
analogSettingRaw: [Int!],
digitalStatusRaw: [Int!],
# scaled using primary and common.
# array since we need to also support array devices.
analogReading: [Float!],
analogSetting: [Float!],
# only primary scaling
analogReadingPrimary: [Float!],
analogReadingSetting: [Float!]
# digital reading with descriptions
digitalStatus: [Status!]
}
type Status {
bitName: String,
status: Boolean
}
Representing a device as an object in our schema has a few main advantages:
-
The schema describes exactly what data you can query for our devices. With DRF strings, users have to lookup the DRF string specification to figure out what/how to request properties of a device.
-
In the current schema, the syntax of a request is not linked to the semantics of the data we are requesting. For instance, if we ask for G:AMANDA, the semantics of this request dictates we should return the scaled float analog readback for the device. However, there is nothing in our current schema structure that links the request to what type of data we would expect from the response. Instead, users have to make this link themselves. An example of the semantics of our DRF request being out of sync with the syntax of our request would be something like:
{
acceleratorData(deviceList: ["G:AMANDA"]){
... on Raw {
rawValue
}
}
}
which is a syntactically correct request, but is semantically incorrect as we are requesting the scalar readback of G:AMANDA and only handling raw responses.
- This proposed schema coupled with most GraphQL clients provides clear feedback to the user about misformatted requests. Right now, if the user misformats their DRF string, they just get a network error (at least, as tested on acsys-proxy.fnal.gov:8001/acsys). Even if we did return some sort error message for misformatted DRF strings that precisely indicate where the user went wrong, this is something that we have to build and maintain ourselves and is separate from the request validation that GraphQL performs.
Supplying data source and FTD
The part of our DRF string that indicates the datasource/FTD to use for our request could also be modeled as a GraphQL object.
Here is a possible way of doing it:
input Request {
name: String,
source: Source,
startTime: Int,
endTime: Int
}
input Source @oneof {
dataLoggerSource: DataLoggerSource,
liveSource: LiveSource
}
input DataLoggerSource{
ftd: FTD!,
node: Loggers # if provided, will only look for the provided dl node for the device + ftd
}
enum Loggers {
MInj,
Mau,
Inst,
# ... and so on. Could have a build script pull this info from the db
}
input LiveSource{
ftd: FTD
}
input FTD @oneof {
periodic: Periodic,
onEvent: OnEvent
}
input Periodic {
frequency: Int # frequency in hz
}
input OnEvent {
event: Event,
delay: Int, # delay in milliseconds
forceHardware: Boolean,
}
enum Event {
EVENT_00,
EVENT_01
#
# ... and so on, I think enum works well for this since then graphQL will
# check the user has not typed in something that is not a valid TCLK event, plus
# we can rip the event descriptions from https://www-bd.fnal.gov/controls/hardware_vogel/tclk.htm
# and use this for the description of each event enum variant, which makes the schema nicely self-
# documented.
}
Putting it all together
One downside of this approach is that we don't get as much flexibility when it comes to building a list of device requests. This becomes apparent when we try to figure out what the root Query and Subscription type should look like. Should the field only return the information for a single device, like this?
type Query {
device(request: Request): Device
}
In this case, if the user wants to get a list of devices back they would have to do something like this:
{
AMANDA_AN : device(request: {G:AMANDA, ...} ){
analogReading
}
AMANDA_DIG : device(request: {G:AMANDA, ... }) {
digitalStatus
}
# ...and so on and so forth
}
which requires the user to alias the device field name.
The other option could be for the device field to return a list, like this.
type Query {
device(request: Request): [Device!]
}
But this carries the restriction that we can't pick out a distinct field for each of these devices without again resorting to aliasing the device field.
Back end structure
The other issue with this approach is that, to my knowledge, the DPM is build around accepting a list of devices. This list can be started and stopped which starts and stops the acquisition of all of the devices the list contains.
Both of the options for Query described above run the risk of creating a bunch of lists that really should be consolidated into one list, since, as I understand it, the GraphQL server implementation will run the resolver for each query on the device field in parallel, meaning that each invocation of the resolver will create a new list in the DPM. Hence, for a request of 100 devices, we would generate 100 lists all with 1 device. This doesn't seem ideal given the current architecture of the DPMs.
I believe using Dataloaders which is supported by our GraphQL server implementation should fix this by allowing us to batch our requests together.
Please poke holes in this design!
I had some suggestions when looking through the GraphQL API as it exists now.
My main comment is that our API still feels a little "REST-like" due to the fact we are using DRF strings in our requests. Instead, I propose we move away from DRF strings and instead model them in GraphQL's type system.
Querying device properties
Instead of relying on the DRF string to denote what specific property of a device the request is asking for, we should instead represent a device as a GraphQL object.
This could look something like this:
Representing a device as an object in our schema has a few main advantages:
The schema describes exactly what data you can query for our devices. With DRF strings, users have to lookup the DRF string specification to figure out what/how to request properties of a device.
In the current schema, the syntax of a request is not linked to the semantics of the data we are requesting. For instance, if we ask for G:AMANDA, the semantics of this request dictates we should return the scaled float analog readback for the device. However, there is nothing in our current schema structure that links the request to what type of data we would expect from the response. Instead, users have to make this link themselves. An example of the semantics of our DRF request being out of sync with the syntax of our request would be something like:
{ acceleratorData(deviceList: ["G:AMANDA"]){ ... on Raw { rawValue } } }which is a syntactically correct request, but is semantically incorrect as we are requesting the scalar readback of G:AMANDA and only handling raw responses.
Supplying data source and FTD
The part of our DRF string that indicates the datasource/FTD to use for our request could also be modeled as a GraphQL object.
Here is a possible way of doing it:
Putting it all together
One downside of this approach is that we don't get as much flexibility when it comes to building a list of device requests. This becomes apparent when we try to figure out what the root
QueryandSubscriptiontype should look like. Should the field only return the information for a single device, like this?In this case, if the user wants to get a list of devices back they would have to do something like this:
{ AMANDA_AN : device(request: {G:AMANDA, ...} ){ analogReading } AMANDA_DIG : device(request: {G:AMANDA, ... }) { digitalStatus } # ...and so on and so forth }which requires the user to alias the
devicefield name.The other option could be for the
devicefield to return a list, like this.But this carries the restriction that we can't pick out a distinct field for each of these devices without again resorting to aliasing the
devicefield.Back end structure
The other issue with this approach is that, to my knowledge, the DPM is build around accepting a list of devices. This list can be started and stopped which starts and stops the acquisition of all of the devices the list contains.
Both of the options for
Querydescribed above run the risk of creating a bunch of lists that really should be consolidated into one list, since, as I understand it, the GraphQL server implementation will run the resolver for each query on thedevicefield in parallel, meaning that each invocation of the resolver will create a new list in the DPM. Hence, for a request of 100 devices, we would generate 100 lists all with 1 device. This doesn't seem ideal given the current architecture of the DPMs.I believe using Dataloaders which is supported by our GraphQL server implementation should fix this by allowing us to batch our requests together.
Please poke holes in this design!