feature(gatsby-source-drupal): Use list of UUIDs generated by Drupal to fetch content individually #32131
Conversation
gatsbot
bot
added
the
status: triage needed
| const isTranslatable = languageConfig.translatableEntities.some( | ||
| entityType => entityType === type | ||
| ) | ||
| const listResponse = await worker([ |
There was a problem hiding this comment.
you should be using the requestQueue here and other places
There was a problem hiding this comment.
Yeah I figured out how to do that yesterday. Feels weird pushing a return value from a queue.push to a queue.push but here we go :)
| headers, | ||
| } | ||
| ]) | ||
| const listResponseBody = JSON.parse(listResponse.body) |
There was a problem hiding this comment.
For whatever reason, passing responseType: 'json' to got and letting it do the JSON.parse results in some very weird artifacts in some environments. It worked 100% of the time on my local builds but doing a build in CI failed.
There was a problem hiding this comment.
it's almost certainly that somehow your CI version isn't getting the right version of got. Probably because you're running this in a local plugin which means that it's whatever version of got is resolving in your site which because Gatsby's dependency tree unfortunately contains older versions of Got, isn't going to be predictable atm. Probably best for testing to publish the module to a temp npm package so that it can resolve its own dependencies correctly.
| requestQueue.push([ | ||
| urlJoin(baseUrl, language, apiBase, `/${entityType}/${entityBundle}/${entityUuid}`), | ||
| { | ||
| headers: shouldUseAuth ? headers : undefined |
There was a problem hiding this comment.
This dumps all the other headers if you don't want auth on the entity, need a better way to do this.
| } = pluginOptions | ||
| const { createNode, setPluginStatus, touchNode } = actions | ||
|
|
||
| // Update the concurrency limit from the plugin options | ||
| requestQueue.concurrency = concurrentAPIRequests | ||
|
|
||
| if (typeof basicAuth.username === 'string' && typeof basicAuth.password === 'string') { |
There was a problem hiding this comment.
This is to work around: sindresorhus/got#1169
| // based on relationships. The expanded data is exposed as `included` | ||
| // in the JSON API response. | ||
| // See https://www.drupal.org/docs/8/modules/jsonapi/includes | ||
| if (d.body.included) { |
There was a problem hiding this comment.
We're losing "included"
LekoArts
changed the title
LekoArts
added
topic: source-drupal
Another idea here — we could just auto-retry with authentication on 403 failures. Maybe learn automatically which content-types need auth too and store that in the plugin cache for subsequent fetching. |
Problem is: 403 != content type requires auth. Sometimes a 403 just means that the content isn't published. There might be a difference between these 2 cases (403 vs 401) but not 100% sure. |
|
ah right — yeah 401 is what we'd retry with auth |
|
The |
That's probably a good way to handle it. Some demarcation (maybe keyed differently or with a |
LekoArts
added
the
topic: source-plugins
Description
As per discussions with @KyleAMathews we've been considering using the JSON:API individual content page eg. /jsonapi/node/article/1234 because its cacheability is much better than the listing endpoint. This means that for requests which can be cached the response time is increased on average 20x. This PR requires you have the latest patch from this issue: https://www.drupal.org/project/gatsby/issues/3220713 installed which enables the
/gatsby/content-listendpoint.There's still a few questions surrounding this:
fetchIndividualEntities: trueingatsby-configfastbuildsDoing some testing with my own Drupal site I saw:
This cache-hit-ratio is not going to fluctuate as much as it does with the current source plugin because node saves won't purge all of these cache entries only the relevant pages to the particular node that was saved.