-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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(Turborepo): wire file hashing to grpc server #7868
feat(Turborepo): wire file hashing to grpc server #7868
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
6 Ignored Deployments
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
🟢 Turbopack Benchmark CI successful 🟢Thanks |
🟢 CI successful 🟢Thanks |
babe8ee
to
1ba0d7a
Compare
6d7d52a
to
ceea917
Compare
1ba0d7a
to
de642b3
Compare
ceea917
to
2e706a9
Compare
ac229a4
to
93d74a7
Compare
2e706a9
to
cc9adaa
Compare
93d74a7
to
8478cd5
Compare
cc9adaa
to
4e290a9
Compare
8478cd5
to
232ce61
Compare
4e290a9
to
ea03d90
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one clarification but otherwise looks good
} | ||
} | ||
}); | ||
let mut hash_object = match hash_object { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not very important but you could probably use or_else
or unwrap_or_else
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked at doing that, but it gets awkward since we're starting with an Option
, but the fallback calculation produces a Result
where we want to short-circuit, but the overall closure returns an Option<Result<_, _>>
. ?
doesn't really work in that context, so you end up with the match
either way.
// active tokio context. Constructing it directly in | ||
// the rayon thread doesn't provide one and will crash at runtime. | ||
handle | ||
.block_on(async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little confused by the docs for this method. It says the method panics "if called within an asynchronous execution context". Does that mean if it's called in an async function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Essentially, yes, if it's running from a task managed by an async runtime, it will panic. In this case, we're running from the rayon threadpool.
Description
Testing Instructions
Existing integration test suite
Closes TURBO-2736