Comments (12)
ctx.(fx.Context)
won't work if anything between the UberFx middleware and
WrapUnary calls context.WithValue
on the context because the returned ctx
will not have the added fx.Context
functions anymore.
from fx.
There may be performance implications to using a struct, since we'll be passing the struct by value, and when a by-value object is converted to an interface, it requires an allocation. Since the fx.Context
will be passed to methods that expect a context.Context
interface, it's possible that each of these will require an extra allocation to do the interface conversion.
We should ensure there's no performance hit if we want to do this.
from fx.
cc @anuptalwalkar who is doing the implementation
from fx.
And yes, the interface conversion was the primary motivation for us using an interface in he first place.
from fx.
If this affects perf negatively, an interface can still be used to get
context.Context
compatibility provided that UberFx maintains control of all
implementations by adding a private method to the interface and providing a
public function to upgrade a context.Context
back to an fx.Context
.
package fx
type Context interface {
context.Context
// unexported function to ensure only this package implements this
// interface.
fx()
// ...
}
func Upgrade(context.Context) Context { // Better name TBD
return &context{...}
}
func NewContext(context.Context, Host) Context {
// ...
}
type context struct {
// ..
}
func (c *context) fx() {}
func (c *context) Logger() Logger {
return GetLogger(c.Context)
}
This gets rid of the fx.Context -> context.Context
alloc.
from fx.
from fx.
Private method on the interface is a good idea. I'd avoid adding the Upgrade
-- my understanding is that this shouldn't be needed, and the uberfx team actively wants to avoid wrapping.
from fx.
I like the private method idea to keep the implementation restricted.
Is there a performance impact on doing fxCtx := ctx.(fx.Context) ?
from fx.
@anuptalwalkar ctx.(fx.Context)
will not work if context.WithValue
was
used on an fx.Context
.
fxctx := fx.NewContext(...)
ctx := context.WithValue(fxctx, ...)
fxctx2 := ctx.(fx.Context) // this will panic
The two solutions recommended here would solve that.
@prashantv The upgrade function is needed to solve the problem mentioned
above. On second thought, it doesn't have to be public, but we definitely need
it.
Imagine that an UberFx InboundMiddleware adds UberFx information to incoming
contexts:
func (m *ctxMiddleware) Handle(
ctx context.Context, ..., h transport.UnaryHandler,
) error {
fxctx := fx.NewContext(ctx, m.Host)
return h.Handle(fxctx, ...)
}
We get this context as a context.Context
at the encoding layer. We need to
convert this back to an fx.Context
to call the user handler. This would be
done with UberFx's Thrift function wrapper.
func WrapUnary(h fxthrift.UnaryHandlerFunc) yarpcthrift.UnaryHandler {
return func(ctx context.Context, ...) error {
fxctx := upgrade(ctx)
return h(fxctx, ...)
}
}
from fx.
I am following the same approach in the refactoring. Well, ctx.(fx.Context)
works in WrapUnary since it is injected in the inboundMiddleware (unless we write a middleware to do WithValue in between the calls), but in this scenario upgrade works just as well. We definitely don't want to do unnecessary wrapping.
from fx.
from fx.
Added benchmarks for Context conversion - #131
from fx.
Related Issues (20)
- fx.Decorate does not add a missing object to the module HOT 3
- When combining fx.As to annotate return type as interface with optional parameters nil value check is not working as expected HOT 1
- Startup order isn't preserved when using modules HOT 3
- Example test failing in master branch when port 8080 is already in use (89f4a90) HOT 3
- fxevent.Logger: Silence until there's an error
- Slog adapter for fx HOT 4
- Release method HOT 10
- fxtest.Lifecycle: Enforce timeout
- Impossible to inject struct that is part of a group by itself HOT 2
- Support Multiple ParamTags? HOT 1
- signalReceivers should call `Signal.Stop(recv.signals)` when stopped
- Release v1.22.0
- Private supplies HOT 1
- Support multiple ResultTags annotations
- Opt-out of signal handling HOT 12
- Add constructor runtime to fxevent.Run events
- Ability to use fx.As and fx.ResultTags for the same Provider HOT 1
- Deadlock in `signalReceivers.Stop` HOT 6
- Decorate does not work if inside fx.Module and Populate is called from outside Module HOT 6
- Quieter fxtest logger
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from fx.