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

feat(gatsby-source-drupal): Provide proxyUrl in addition to baseUrl to allow using CDN, API gateway, etc. #36819

Merged
merged 19 commits into from
Nov 22, 2022

Conversation

Vacilando
Copy link
Contributor

@Vacilando Vacilando commented Oct 13, 2022

Description

Provide param proxyUrl to be used to fetch the Drupal JSON API resources. It can be a CDN or API Gateway URL.

In my particular situation - fetch from a Drupal server (32 GB RAM, 8 vCPUs) vs fetch from a CDN (CloudFront) improves the "Fetch all data from Drupal" build phase by a factor of 10. (17s instead of 170s for almost 100k Drupal nodes)

Documentation

Documentation is provided in README in section "CDN".

Related Issues

Fixes / addresses:

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 13, 2022
@Vacilando Vacilando changed the title feat(gatsby-source-drupal): Provide proxyUrl in addition to baseUrl to allow using CDN, API gateway, etc. #36811 feat(gatsby-source-drupal): Provide proxyUrl in addition to baseUrl to allow using CDN, API gateway, etc. Oct 13, 2022
@Vacilando
Copy link
Contributor Author

There are 7 failed tests. They seem unrelated to the pull request, am I right?
Is there anything else I can do to make this merge-worthy? :-)

Copy link
Contributor

@KyleAMathews KyleAMathews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense and looks good — can you add a test or two for this before we merge it?

@KyleAMathews
Copy link
Contributor

on the failed tests — yeah we just switched yesterday master to point to the Gatsby v5 beta code and tests are completely passing yet for v5. The only ones this PR needs to pass are the unit tests (where the gatsby-source-drupal tests are run).

Co-authored-by: Kyle Mathews <mathews.kyle@gmail.com>
@Vacilando
Copy link
Contributor Author

@KyleAMathews thanks for the approving words and the READMe suggestions - already committed.

I've never written tests for Gatsby, not sure where to begin or end. Will start looking around but any suggestion of a minimal test you would like to see / any pointers how to go about it will be much appreciated!

@KyleAMathews
Copy link
Contributor

The tests are here: https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-source-drupal/src/__tests__/index.js

You can run them locally by navigating to the gatsby-source-drupal directory and running npx jest . --watch.

Probably the easiest way to test this is to copy an existing test but with a proxyUrl in the config and then mock the url replacement (you'll need to make it a standalone function to do this I believe) so you can then check that it was called with the variables you expect and returns the right result. Hope that helps!

@Vacilando
Copy link
Contributor Author

@KyleAMathews - thanks for the testing tips. I tried several times from scratch setting up the testing environment according to https://www.gatsbyjs.com/docs/how-to/testing/unit-testing/ but no luck. So did not even get over to writing the tests.

I get errors like
image
and cannot run any tests

Perhaps the instructions are outdated, or it has something to do with Gatsby v5 beta, not sure.

Can someone help here with writing a test, or can we merge this without more tests for now. If not I am afraid I have to give it up for some period :-(

NB I run the modified plugin locally and also in Gatsby Cloud, no issues found, the speedup over direct pull from a powerful Drupal server is 10x, so it's definitely a useful feature.

@Vacilando
Copy link
Contributor Author

Hi again, I need help with the tests / merging this little feature.

I am writing a blog article about this performance improvement for Drupal backends and would very much like to mention the version of gatsby-source-drupal where this will have been implemented.

@TylerBarnes
Copy link
Contributor

@KyleAMathews @Vacilando I took a crack at adding a test. lmk if that looks right

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants