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

Add endpoint to refresh external data #3054

Closed

Conversation

PepperTeasdale
Copy link
Contributor

@PepperTeasdale PepperTeasdale commented Nov 28, 2017

Fixes #2847 . The external source data can be refreshed by making a POST request to
the __refresh endpoint. If the env var GATSBY_REFRESH_TOKEN is present
there needs to be a matching Authorization header. This post request
is wrapped for convenience with the command gatsby refresh which uses
localhost:8000 by default, but allows for a host and port to be
specified in the same way as gatsby develop.

@gatsbybot
Copy link
Collaborator

gatsbybot commented Nov 28, 2017

Deploy preview ready!

Built with commit 64c6d2a

https://deploy-preview-3054--gatsbygram.netlify.com

@gatsbybot
Copy link
Collaborator

gatsbybot commented Nov 28, 2017

Deploy preview ready!

Built with commit 64c6d2a

https://deploy-preview-3054--using-drupal.netlify.com

@bjrn
Copy link

bjrn commented Nov 28, 2017

This would indeed be very useful — rather than having every plugin coming up with a way to refresh data, having a gatsby preview would enable an easy way to set up a preview server for a project, and have it update on changes, regardless where the data comes from; contentful, prismic etc.

@PepperTeasdale PepperTeasdale changed the title WIP add endpoint to refresh external data PROPOSAL add endpoint to refresh external data Nov 29, 2017
@PepperTeasdale PepperTeasdale changed the title PROPOSAL add endpoint to refresh external data [PROPOSAL] add endpoint to refresh external data Nov 29, 2017
@PepperTeasdale
Copy link
Contributor Author

@KyleAMathews curious if this is something you would consider supporting before getting deeper into development. If not, could we expose some way to add a community-supported preview server extension (or if this is already supported, please let us know how)?

@KyleAMathews
Copy link
Contributor

rather than having every plugin coming up with a way to refresh data

That was never the idea — all source plugins should be idempotent i.e. Gatsby can call them whenever it wants. That's why I've been suggesting adding an extension to gatsby develop that lets developers trigger refetching data from sources.

@KyleAMathews
Copy link
Contributor

Exactly like this PR is doing :-)

@KyleAMathews
Copy link
Contributor

@PepperTeasdale I think a built-in preview "server" makes a ton of sense

@PepperTeasdale
Copy link
Contributor Author

Awesome! I'll start by fleshing out this webhook to refresh the dev server, since its something we'd probably like in both dev and preview. Then, preview feature would be more focused on a similar webpack config, but without HMR and more efficient source maps, etc.

@PepperTeasdale PepperTeasdale force-pushed the refresh-data-sources branch 2 times, most recently from c4685aa to 213981d Compare December 4, 2017 18:21
@PepperTeasdale PepperTeasdale changed the title [PROPOSAL] add endpoint to refresh external data Add endpoint to refresh external data Dec 4, 2017
@PepperTeasdale
Copy link
Contributor Author

PepperTeasdale commented Dec 4, 2017

@KyleAMathews 👀 please :) No longer a WIP. I would like to add some tests, but wasn't seeing where this kind of stuff was being tested in the project. If you have any ideas on that, please let me know.

The external source data can be refreshed by making a POST request to
the `__refresh` endpoint. If the env var GATSBY_REFRESH_TOKEN is present
there needs to be a matching `Authorization` header. This post request
is wrapped for convenience with the command `gatsby refresh` which uses
localhost:8000 by default, but allows for a host and port to be
specified in the same way as `gatsby develop`.
Copy link

@bjrn bjrn left a comment

Choose a reason for hiding this comment

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

Great work! Covers all things we've been looking for in a preview server setup, right down to elegant handling of the refresh_token. And with a lot less code changes than I had imagined!

@KyleAMathews is there anything preventing this from getting merged? I guess this feature falls under advanced usage, but is more documentation needed? An example setup?

* If no GATSBY_REFRESH_TOKEN env var is available, then no Authorization header is required
**/
app.post(`/__refresh`, (req, res) => {
if (req.headers.authorization === process.env.GATSBY_REFRESH_TOKEN) {
Copy link

Choose a reason for hiding this comment

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

nitpick or misunderstanding perhaps, but if process.env.GATSBY_REFRESH_TOKEN is undefined, shouldn't refresh happen regardless of what is passed in res.headers.authorization?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for having a look! You point about the authorization headers makes sense. Also intend on adding appropriate documentation. Just wanted to handle any requested changes to the implementation details before writing it out (eg if the refresh token is defined as a command line argument when starting the server, rather than an env var).

Copy link

Choose a reason for hiding this comment

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

I think I'd personally vote for it to be an env var

@PepperTeasdale
Copy link
Contributor Author

I was thinking about it this weekend, and it might make sense to open this up as a hook for plugins, something like onDataRefresh. The default behavior might be to just refetch and build everything, but allow the user to define plugins that can be smarter about how to handle the POST request, based on the data received.

Nothing here would prevent future development of something like that.

@@ -105,6 +105,25 @@ function buildLocalCommands(cli, isLocalSite) {
handler: getCommandHandler(`develop`),
})

cli.command({
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you assuming that you'd have gatsby develop running and then run this in another terminal tab? If so, this wouldn't work as Gatsby keeps the node data in memory — we need to add the ability to pass commands to the running gatsby develop process e.g. press "r" and a refresh would be triggered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for looking! This does work (in my local tests, at least), as it makes a post request to the port the server is running on, hitting an endpoint that calls sourceNodes().

If I understand correctly, you envision that the process running gatsby develop is also listening to stdin and updating based on input there, (eg: pressing "r" in the terminal window). I'm not familiar with any programs that do that, but I can totally give that a shot.

Even without the refresh command, would you be open to the dev server accepting a webhook to reload the data? That is my use case and what some others commenting on the related issues are looking for.

Copy link

Choose a reason for hiding this comment

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

ah, pressing r as in reload (kind of like how Jest CLI works?) could be nice for local development, but I agree with @PepperTeasdale that the main use case would be to use the webhook … since it's just a POST, building a trigger that fits the requirements on top of it would be trivial.

@PepperTeasdale
Copy link
Contributor Author

@KyleAMathews Just bumping to see if you would be interested in supporting the webhook portion of this PR. And if you would, what changes would be required for a merge? Thanks!

@KyleAMathews
Copy link
Contributor

@PepperTeasdale yeah! Let's ship it first w/o CLI command so we can test it w/o confusing people initially for what it's for. That gives y'all more flexibility for trying stuff early on. Later we can add the CLI command & documentation, etc.

@KyleAMathews
Copy link
Contributor

Dark deploy :-)

@PepperTeasdale
Copy link
Contributor Author

PepperTeasdale commented Dec 20, 2017

Thanks @KyleAMathews ! Closing this and putting in the slimmed down PR at #3293

@wysher
Copy link

wysher commented Mar 11, 2018

@PepperTeasdale, @KyleAMathews
What about mentioned docs? :) Or maybe some simple example how to use it.

@PepperTeasdale
Copy link
Contributor Author

@wysher I need to make a PR to remove this code. It broke after a big fix went in. We ended up abandoning this path in our efforts to make a preview server.

@camsloanftc
Copy link

@PepperTeasdale - when you say you're going to submit a PR to remove this code, which code are you referring to? The code from #3293? I am just curious because we are about to set up a preview server and we were planning to use the refresh endpoint you added in that PR. Could you let me know if that will be removed soon then? It will change our course of action.

@PepperTeasdale
Copy link
Contributor Author

@camsloanftc I do think that #3293 should be removed
@KyleAMathews merged that PR to allow us to test that idea, but it didn't pan out. See my comment on the original issue for more context. I'm sorry I didn't think to do this sooner. You could certainly investigate this route further for your preview server, but I don't think it would work out the box (at least last time I checked).

@PepperTeasdale
Copy link
Contributor Author

Although @Jrousell said they got it working last week, so perhaps I am wrong about this.

@camsloanftc
Copy link

I had it working before as well, with the exception of some Contentful data that seemed to be stuck in cache when attempting to delete content. I will try to set it up again to see what the status is.

@camsloanftc
Copy link

I was just testing out this feature again and had it working fairly well. I made a video of the process. I will summarize it here though:

I added the environment variable: ENABLE_GATSBY_REFRESH_ENDPOINT=true to my .env file

Then I setup a webhook in Contentful to access my local machine. I used ngrok to make a secure tunnel to do so. This is just for demonstration, usually this would point to a real server.

Once I run gatsby develop on my machine, in Contentful I was able to update content, titles, etc.
I could rearrange a list of linked fields, or delete linked items.

Where I ran into trouble was archiving or deleting items. It does not seem to bust through what is stored in the .cache folder. I think this has been referenced in a separate issue, (#3495 maybe).

If there is interest in seeing a video of this in action, I have it posted here.

It seems to me like we may not need to remove this code, but figure out the issues with the cache.

@KyleAMathews
Copy link
Contributor

@Khaledgarbaya
Copy link
Contributor

👋 @camsloanftc @KyleAMathews that can be easily done by https://www.contentful.com/developers/docs/references/content-delivery-api/#/reference/search-parameters/links-to-entry
You can query contentful for all entries that reference the now deleted entry/asset

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

Successfully merging this pull request may close these issues.

None yet

7 participants