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

Remove image downloading out of query-running for contentful #24220

Closed
wardpeet opened this issue May 19, 2020 · 9 comments
Closed

Remove image downloading out of query-running for contentful #24220

wardpeet opened this issue May 19, 2020 · 9 comments
Labels
not stale topic: source-contentful Related to Gatsby's integration with Contentful

Comments

@wardpeet
Copy link
Contributor

Summary

Currently contentful does base64 generating during query-running. It reduces the amount of data being downloaded and process by contentful but we only run 4 queries at a time so this slowdown query-running.

const getBase64Image = imageProps => {
if (!imageProps) return null
const requestUrl = `https:${imageProps.baseUrl}?w=20`
// Note: sha1 is unsafe for crypto but okay for this particular case
const shasum = crypto.createHash(`sha1`)
shasum.update(requestUrl)
const urlSha = shasum.digest(`hex`)
// TODO: Find the best place for this step. This is definitely not it.
fs.mkdirSync(CACHE_IMG_FOLDER, { recursive: true })
const cacheFile = path.join(CACHE_IMG_FOLDER, urlSha)
if (fs.existsSync(cacheFile)) {
// TODO: against dogma, confirm whether readFileSync is indeed slower
return fs.promises.readFile(cacheFile, `utf8`)
}
return new Promise(resolve => {
base64Img.requestBase64(requestUrl, (a, b, body) => {
// TODO: against dogma, confirm whether writeFileSync is indeed slower
fs.promises.writeFile(cacheFile, body).then(() => resolve(body))
})
})
}

Motivation

Make query-running faster

@wardpeet wardpeet added type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement. topic: source-contentful Related to Gatsby's integration with Contentful labels May 19, 2020
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label May 19, 2020
@wardpeet wardpeet removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label May 19, 2020
@github-actions
Copy link

github-actions bot commented Jun 9, 2020

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Jun 9, 2020
@axe312ger
Copy link
Collaborator

Related: #24679

@axe312ger
Copy link
Collaborator

This will be fixed in the next major version as it requires potentially breaking refactoring to reduce the network requests.

See #25249

@github-actions
Copy link

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 days of inactivity. It’s been at least 20 days since the last update here.
If we missed this issue or if you want to keep it open, please reply here. You can also add the label "not stale" to keep this issue open!
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

@github-actions github-actions bot added the stale? Issue that may be closed soon due to the original author not responding any more. label Jul 20, 2020
@github-actions
Copy link

Hey again!

It’s been 30 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it.
Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to reopen this issue or create a new one if you need anything else.
As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks again for being part of the Gatsby community! 💪💜

@axe312ger
Copy link
Collaborator

This ticket is directly related #24679 (comment)

@axe312ger axe312ger reopened this Aug 12, 2020
@axe312ger axe312ger added not stale and removed stale? Issue that may be closed soon due to the original author not responding any more. labels Aug 12, 2020
@axe312ger
Copy link
Collaborator

@wardpeet

Hey there,

I wonder how you'd achieve that. Creating these previews is based on the actual query result, we do not need to download 20px variants for all images stored in Contentful, we "just" have to do it for these who are actually rendered with gatsb(-plugin)-image.

Alternatively we might have some king of separate cache for these, that way we could download previews beforehand, store them in the folder and only delete/replace when the actual file changes. Might add some more network requests to the initial load, but should save a lot later on.

What have been your thoughts when opening this?

Benedikt

@axe312ger
Copy link
Collaborator

@wardpeet I am still certain that we need to download these while running queries and we can not solve this issue.

Should we close it?

@LekoArts LekoArts removed the type: feature or enhancement Issue that is not a bug and requests the addition of a new feature or enhancement. label Aug 9, 2021
@LekoArts
Copy link
Contributor

We can close it as per Benedikt 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not stale topic: source-contentful Related to Gatsby's integration with Contentful
Projects
None yet
Development

No branches or pull requests

3 participants