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

Retain request context values while detaching to avoid cancellation. #342

Closed
wants to merge 1 commit into from

Conversation

innovate-invent
Copy link

@innovate-invent innovate-invent commented Jan 3, 2020

Copypasta of @Acconut solution to #315

@Acconut
Copy link
Member

Acconut commented Jan 20, 2020

Thanks for the PR. Can you add a test to ensure that the context values from the HTTP request are actually accessible?

@dschmidt
Copy link

dschmidt commented Apr 8, 2020

This only makes the original ctx available for DataStores, right?

So there's still no way to access the context from HookEvents right? Is that by design or just missing? If by design, what's the reason for that?

edit:
So right now the only way to pass additional data to the HookEvent would probably be by messing with the headers?

butonic added a commit to butonic/tusd that referenced this pull request Apr 17, 2020
When trying to embed the tusd handler these interfaces are missing, AFAICT.
Also see https://github.com/cs3org/reva/pull/661/files#diff-1661b4643c6e31dcd318877ae6181470 for a working version, which is a fork of the handler I'd like to get rid of, especially seeing tus#342
@Acconut
Copy link
Member

Acconut commented May 1, 2020

This only makes the original ctx available for DataStores, right?

Yes, correct.

So there's still no way to access the context from HookEvents right? Is that by design or just missing? If by design, what's the reason for that?

Yes, it's not really a design decision but grew naturally that way. The hook system was built before contexts existed in Go, so they were not included in the design of the hooks.

So right now the only way to pass additional data to the HookEvent would probably be by messing with the headers?

You can do that. But it depends on what additional data you want to pass on and where it comes from.

@Acconut
Copy link
Member

Acconut commented Aug 23, 2023

Thanks you for your contribution! In tusd v2, the request context will be accessible in the hooks and stores. Please see #986 for more details and #672 for the development of v2.

@Acconut Acconut closed this Aug 23, 2023
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.

3 participants