Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse -api graphql schema and determine what is available #151

Open
fnoop opened this issue Feb 29, 2020 · 13 comments
Open

Parse -api graphql schema and determine what is available #151

fnoop opened this issue Feb 29, 2020 · 13 comments
Milestone

Comments

@fnoop
Copy link
Member

fnoop commented Feb 29, 2020

When an api is defined and thus a new graphql client created, download the schema, parse and work out what is available and what is not.

@fnoop
Copy link
Member Author

fnoop commented Feb 29, 2020

Currently, when the root component is mounted, all defined apis are iterated through and a graphql client created for each defined api by calling CoreApi.createClient(). In createClient(), it stores the client state in vuex:

// Add a vuex apis entry
          this.$store.commit('addApi', {
            title: api,
            value: { ...{ state: false, auth: false, icon: null, uuid: null }, ...clientdata }
          })

The 'state' is set to false by default, and only set to true if a Status message is received from the api which marks it as alive.

Periodically (once per second), the root component (src/App.vue) fires a timer which scans each client and if it hasn't been seen active within the last ten seconds it marks the 'state' as false again.

Then, a component will add GraphQL queries/subscription that it needs, by calling something like:
this.createQuery('NavSatFix', navSatFixQuery, api, 'navSatFixData', !api.state)
It passes the 5th parameter !api.state, which corresponds to the skip parameter of createQuery(). This means that if the api state is false (ie. if it hasn't yet successfully received a single Status message), then the skip state of the query is set to True - in other words all calls or blocked while skip is True - it's disabled.

This is quite effective, but limited. It assumes that if it receives a Status message, all other messages are available. So when Status is received, it turns on all the various Mavlink queries/subscriptions, even if they're not enabled in -api. This will lead to failures and errors. We can't do a similar thing for any or all of the mavlink and other graphql messages, because we have to know that they're available in order to create the query, activate it and listen for messages.

We assume that the Status message is always available - it's effectively the core metadata message of the -api instance and must always be available.

There are two methods to work round this problem:

  • Add a modules list to the Status message, which is updated by -api to include the active modules available. The Status message is a subscription, so the -web client will automatically update immediately upon receiving a message update from -api and reflect the changes in the api metadata held in vuex.
  • Retrieve and parse graphql schema, work out what is available, and store this in the api metadata in vuex.

@SamuelDudley
Copy link
Member

SamuelDudley commented Mar 1, 2020

Naively, can we replace !api.state with !verifyQuery(navSatFixQuery, api)?

Assuming we fetch the schema from /schema as part of the client bootstrap procedure we currently have everything we need to verify if it is okay to make a request client side at runtime.

The status message can contain a schema hightide timestamp. If the timestamp is greater than the one stored in the api metadata:

  • re-fetch the schema from /schema
  • invalidate any stored validation metadata with clearVerifiedQueries(api)

The next time the query is executed verifyQuery() is checked against the new schema. If it fails, then skip is set for that query, otherwise everything keeps going.

@fnoop
Copy link
Member Author

fnoop commented Mar 1, 2020

Naively, can we replace !api.state with !verifyQuery(navSatFixQuery, api)?
Yes, absolutely!

What you're describing will work, but it's awfully complicated and requires multiple pulls/parses of the remote schema.

Isn't it just easier to add a schema/modules field to the Status message that keeps an up to date list of what is available? This will be automatically updated through the Status message subscription in the client, with 1/100th of the code and effort, and will be more immediate and faster without the remote schema pull/parsing. I get the schema parse is more 'correct', but technically -api should be able to reliably set in Message.schema what is in the schema anyway?

@SamuelDudley
Copy link
Member

SamuelDudley commented Mar 1, 2020

I think it will be much more fragile consuming API data via graphql as you propose (rather than using the published schema). The code to pull and verify the schema is already in place in -web where as I would need to write python code to grab all the graphql info and then a graphql message(s) to discribe the queries. We would then need to somehow match the -web graphql queries against the details published by -api via naming or similar. IMO it has its own set of implementation issues while being less robust.

@SamuelDudley
Copy link
Member

If you are agreeable to let me do the leg work on this issue I'll happily go down the python route if / when the JavaScript route fails spectacularly.

@fnoop
Copy link
Member Author

fnoop commented Mar 1, 2020

Sure, your ideas usually work out better than mine anyway!!

@fnoop
Copy link
Member Author

fnoop commented Mar 2, 2020

verifyQuery() needs the gql document which isn't always available. For example in plugins/core/CoreApi.js there is a watcher set that fires whenever the window is not visible, to turn off all the gql queries. When it becomes visible again, it turns them all on. However this now turns on all the 'skipped' queries that verifyQuery was used to disable. There's no way to verify all the queries in all the components because we don't know what gql documents were used to create them.

fnoop added a commit that referenced this issue Mar 2, 2020
@fnoop
Copy link
Member Author

fnoop commented Mar 4, 2020

If verifyQuery() gets a schema which isn't complete or correct (eg. incomplete schema happens during component bootstrap, sometimes is still present during mounted()), then the validation fails and produces errors. I've trapped the errors but it returns incorrectly and causes failure.
But apart from that working well now.
Probably the hashing should be removed in favour of the api key?

@SamuelDudley
Copy link
Member

The hash used for lookup is a hash of the (gql) query string which is stored against the api key. So if we remove the hash we still need a unique, hashable, and reproducible key for each schema defined query.
I was thinking to use the queryKey you created here: https://github.com/goodrobots/maverick-web/blob/master/src/plugins/core/CoreApi.js#L161 but that looks like many unique queries can use the same underling gql code. So.... maybe it makes sense to map the queryKeys to verifiedQuery checks so that gql->hash->verified and queryKey->hash->verified lookups can occur?

Re the incomplete schema during component bootstrap, can we move the schema request to the start of the client creation and run the client creation in an async call (at the same time) as the schema request. Then at the end of the client creation we await the schema call to return (if it hasn't already). This way we know for sure when the modules find the client instance the validation code is ready to go?

@SamuelDudley
Copy link
Member

re

Re the incomplete schema during component bootstrap, can we move the schema request to the start of the client creation and run the client creation in an async call (at the same time) as the schema request. Then at the end of the client creation we await the schema call to return (if it hasn't already). This way we know for sure when the modules find the client instance the validation code is ready to go?

See 91c8846

@fnoop
Copy link
Member Author

fnoop commented Mar 4, 2020

It's all getting a bit complicated.. I've been trying to keep the graphql setup as simple as possible, with as little code as possible, because the setup is very delicate.

What about leaving the schema fetch and any validation requests until after the api has been marked good? When a client is created, a connection is made straight away. It then creates a Status query/subscription for that api, and when it gets a successful Status message back from -api, it marks apis[api].state = true. As long as the api.state==true then it's fully ready for use.

@SamuelDudley
Copy link
Member

Wont the Status query/subscription for that api fail without validation? or are we not validating that?Either way we have to wait for the schema before we can attach any more queries...

@fnoop
Copy link
Member Author

fnoop commented Mar 4, 2020

Yeah currently we just assume that the Status query is available - I think that's a reasonable assumption, a reasonable basic dependency that -api provides Status at all times.

fnoop added a commit that referenced this issue Mar 5, 2020
@fnoop fnoop added this to the 1.2 milestone Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants