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: head support #50

Merged
merged 6 commits into from
Oct 13, 2022
Merged

feat: head support #50

merged 6 commits into from
Oct 13, 2022

Conversation

sppatel
Copy link

@sppatel sppatel commented Oct 7, 2022

Turbo repo introduce support for dry-run to inspect caches and report on whether a cache entry in the local and/or remote cache exists for a given hash. In order to leverage, remote cache implementors must implement a HEAD route to check for the resource. This PR enables this.

See the following for references..

vercel/turborepo#1437
vercel/turborepo#1988

and this article for use cases on leveraging this new info reported by dry-run.

https://medium.com/@sppatel/maximizing-job-parallelization-in-ci-workflows-with-jest-and-turborepo-da86b9be0ee6

@fox1t
Copy link
Collaborator

fox1t commented Oct 7, 2022

Thanks for opening the PR!
One question: aren't HEAD and GET the same thing besides the VERB? If so, we can reuse the same route defined for GET and add the HEAD verb to it.
Also, would you mind adding tests?

@matteovivona matteovivona added the enhancement New feature or request label Oct 7, 2022
@sppatel
Copy link
Author

sppatel commented Oct 7, 2022

Thanks for opening the PR! One question: aren't HEAD and GET the same thing besides the VERB? If so, we can reuse the same route defined for GET and add the HEAD verb to it. Also, would you mind adding tests?

I think they are different because HEAD should not return back a response payload. So the implementation should only check for the existence of the resource and not try to retrive/return it. There would probably be performance implications as well fetching and returning a potentially a large payload that isn't used for the underlying dry run operation - hence prolonging the execution of the command. Yes - I will add tests.

@sppatel sppatel force-pushed the head-support branch 2 times, most recently from 5f8d2f8 to 3f3dd34 Compare October 10, 2022 12:14
@sppatel
Copy link
Author

sppatel commented Oct 10, 2022

@fox1t I've pushed tests but none of the tests pass locally for me (mabye configuration issues but not clear how to setup my env to be able to run locally) - but looks like the actions on CI passed

Copy link
Collaborator

@fox1t fox1t left a comment

Choose a reason for hiding this comment

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

LGTM

@fox1t fox1t merged commit d81d7ef into ducktors:main Oct 13, 2022
@matteovivona
Copy link
Collaborator

@all-contributors please add @sppatel for code

@allcontributors
Copy link
Contributor

@tehkapa

I've put up a pull request to add @sppatel! 🎉

@sppatel
Copy link
Author

sppatel commented Oct 14, 2022

@tehkapa / @fox1t any chance we can cut a release for this - looking to deploy it ASAP.

@matteovivona matteovivona changed the title chore: head support feat: head support Oct 14, 2022
matteovivona added a commit that referenced this pull request Oct 14, 2022
matteovivona pushed a commit that referenced this pull request Oct 14, 2022
# [1.7.0](v1.6.7...v1.7.0) (2022-10-14)

### Features

* trigger release of head support [#50](#50) ([bc3bd8a](bc3bd8a))
@matteovivona
Copy link
Collaborator

@tehkapa / @fox1t any chance we can cut a release for this - looking to deploy it ASAP.

Done.

@matteovivona matteovivona added this to the v1.x.x milestone Feb 6, 2023
TryEverything920609 pushed a commit to TryEverything920609/turborepo-remote-cache that referenced this pull request Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants