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

Make sure linkcontext is passed #207

Merged
merged 1 commit into from
Aug 27, 2021
Merged

Conversation

hannahhoward
Copy link
Collaborator

Goals

LinkContexts passed during selector traversals should always pass through our intercepted loading system so they make it to the base BlockReadOpener / BlockWriteOpener functions on the linksystem passed during setup

Implementation

  • Track everywhere we load in this system, and all locations where we are passing blank LinkContext
  • Add parameters as needed to insure LinkContext is passed.

Ensure all calls that ask for LinkContext are properly passed from function to function
@hannahhoward hannahhoward requested review from adlrocha and gammazero and removed request for gammazero August 26, 2021 06:44
@@ -166,7 +166,7 @@ func (t *traverser) checkState() {
func (t *traverser) writeDone(err error) {
select {
case <-t.ctx.Done():
case t.stateChan <- state{true, err, nil, ipld.LinkContext{}}:
case t.stateChan <- state{true, err, nil, ipld.LinkContext{Ctx: t.ctx}}:

Choose a reason for hiding this comment

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

Just to double check that I am not missing anything. Here we are instantiating a LinkContext exclusively from the process context because there is no parent LinkContext to propagate, right? I ask because building a LinkContext directly from the process context may not propagate certain values in the context. But as this is done at the beginning and the end of the traversal I think it should be fine.

Copy link

@adlrocha adlrocha left a comment

Choose a reason for hiding this comment

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

LGTM. One question, how hard would it be to write a test that checks if context values are actually passed in a traversal? That way we can track that we don't break LinkContext propagation in the future.

We can do it if folks start using this feature extensively, but in case it is straightforward and it is worth doing it now.

@hannahhoward hannahhoward merged commit 9bba3eb into master Aug 27, 2021
@aschmahmann aschmahmann mentioned this pull request Dec 1, 2021
80 tasks
@mvdan mvdan deleted the feat/pass-link-context branch December 15, 2021 14:17
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.

2 participants