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

chore: move run-sift #9549

Merged
merged 19 commits into from
Nov 6, 2018
Merged

chore: move run-sift #9549

merged 19 commits into from
Nov 6, 2018

Conversation

Moocar
Copy link
Contributor

@Moocar Moocar commented Oct 30, 2018

Builds on #9508. Merge that first

Another refactoring PR before loki hits. run-sift has existed in schema up until now. This PR moves it to under the redux folder. The idea is that schema deals with the creation of a schema, but the resolver is code that performs a query against a database. This will make even more sense with loki.

I've also moved node-tracking from schema to db in anticipation that this will eventually be stored in the DB as well.

@Moocar Moocar requested a review from a team as a code owner October 30, 2018 07:45
@Moocar Moocar requested a review from a team October 30, 2018 07:45
@Moocar Moocar requested a review from a team as a code owner November 2, 2018 00:04
@Moocar
Copy link
Contributor Author

Moocar commented Nov 2, 2018

@pieh I just merged master. I really wish github would notify me when a PR I've submitted needs a merge.

@pieh
Copy link
Contributor

pieh commented Nov 5, 2018

@Moocar I meant in here https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby/src/redux/nodes.js#L30-L31 as this will benefit other usages of getNodesByType (outside of running sift) + I'm not sure if it will make sense to use that additional cache when loki would be used (in future)?

@Moocar
Copy link
Contributor Author

Moocar commented Nov 5, 2018

@pieh I'm hesitant to add caching inside the getNodesByType implementation. What if the function is called halfway through sourceNodes when new nodes are still being added?

I think it still makes sense adding it during run-sift until loki is implemented. Since the redux version is quite slow.

@pieh
Copy link
Contributor

pieh commented Nov 5, 2018

I'm hesitant to add caching inside the getNodesByType implementation. What if the function is called halfway through sourceNodes when new nodes are still being added?

That's good point! We could do cache invalidation when we create/update/delete node of given type, but this is out of the scope for this PR.

@pieh
Copy link
Contributor

pieh commented Nov 5, 2018

Was just testing locally, and this actually improve performance a little bit as the caching that You re-added in run-sift yesterday isn't currently used for connection queries ;)

Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

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

Thank You! 🙏

@pieh pieh changed the title Move run sift chore: move run-sift Nov 5, 2018
@Moocar Moocar merged commit 41d53dc into gatsbyjs:master Nov 6, 2018
@Moocar Moocar deleted the move-run-sift branch November 6, 2018 02:48
lipis added a commit to lipis/gatsby that referenced this pull request Nov 6, 2018
* 'master' of github.com:gatsbyjs/gatsby: (63 commits)
  Update how-to-contribute.md to mention the style guide when writing blogs/tutorials (gatsbyjs#9742)
  Added  Tylermcginnis website (gatsbyjs#9619)
  Fix grammar and punctuation (gatsbyjs#9498)
  Fix typo of plugin authoring (gatsbyjs#9737)
  Authentication tutorial - small fixes (gatsbyjs#9738)
  chore: move run-sift (gatsbyjs#9549)
  docs: fix minor typo (gatsbyjs#9730)
  chore(release): Publish
  fix(gatsby-plugin-page-creator): ensure that __tests__ directory is actually ignored (gatsbyjs#9720)
  fix: revert admin redirect (gatsbyjs#9728)
  fix: adjust page order to make nested matchPaths work (gatsbyjs#9719)
  feat(gatsby-plugin-sharp): cache base64 if possible (gatsbyjs#9059)
  chore(release): Publish
  fix(gatsby-plugin-offline): Serve the offline shell for short URLs + use no-cors for external resources (gatsbyjs#9679)
  chore(release): Publish
  fix: ensure babel-preset-gatsby can be used with unit tests (gatsbyjs#9629)
  feat(www): Filter posts by date (gatsbyjs#9400)
  fix(blog): azure blog post url date (gatsbyjs#9718)
  feat(blog): Add post on publishing to Azure (gatsbyjs#8868)
  Emphasize importance of promise return on source-plugin docs (gatsbyjs#9650)
  ...
gpetrioli pushed a commit to gpetrioli/gatsby that referenced this pull request Jan 22, 2019
* Add nodes namespace to support loki feature flag

* naughty console log

* switch for db nodes backend

* move connection out of run-sift

* revert snapshot for gatsby-remark-embed

* typo

* move node tracking into DB

* move run-sift into redux

* use runQuery instead of runSift

* add caching of getNodesByType in run-sift

* fix docs
mwfrost pushed a commit to mwfrost/gatsby that referenced this pull request Apr 20, 2023
* Add nodes namespace to support loki feature flag

* naughty console log

* switch for db nodes backend

* move connection out of run-sift

* revert snapshot for gatsby-remark-embed

* typo

* move node tracking into DB

* move run-sift into redux

* use runQuery instead of runSift

* add caching of getNodesByType in run-sift

* fix docs
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.

4 participants