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

[Feature]: generic Conn context #443

Open
rdmrcv opened this issue Feb 28, 2023 · 4 comments
Open

[Feature]: generic Conn context #443

rdmrcv opened this issue Feb 28, 2023 · 4 comments
Assignees
Labels
breaking changes enhancement New feature or request needs investigation new feature pending development Requested PR owner to improve code and waiting for the result proposal Proposal for this repo
Milestone

Comments

@rdmrcv
Copy link
Contributor

rdmrcv commented Feb 28, 2023

Description of new feature

Would be great if the Conn was a generic interface.

Like:

// ConnG is a generic interface of underlying connection.
type ConnG[Ctx any] interface {
	Reader
	Writer
	Socket

	// ================================== Non-concurrency-safe API's ==================================

	// Context returns a user-defined context.
	Context() (ctx Ctx)

	// SetContext sets a user-defined context.
	SetContext(ctx Ctx)
	...
}

// Conn is an interface that provide a most general Conn interface with an `interface{}` context.
type Conn ConnG[any]

This'll allow users avoid many conversions (and many small allocs) when context used.

This is a breaking change, but in soft manner. Since you can maintain the ConnG and the Conn separately and the Conn will be almost identical to current Conn that use interface{} to a context.
You can get incompatibility here, but mostly in advanced use-cases when some tool rely on the Conn internal representation.

Also, the change will require the golang version bump to 1.18. You can also maintain compatibility if you leave the original Conn implementation and guard it via build tags.

Scenarios for new feature

  • For those who do not use context — nothing changes. They'll still use Conn and nothing should changes for them.
  • For those who use an interface{} as Context. Same as above.
  • For those who want use own context - nothing changes except elimination all of the conversions from an interface{} to the own type.

Breaking changes or not?

Yes

Code snippets (optional)

No response

Alternatives for new feature

None.

Additional context (optional)

Good example of same change can be found here.

@rdmrcv rdmrcv added enhancement New feature or request proposal Proposal for this repo labels Feb 28, 2023
@panjf2000 panjf2000 added pending development Requested PR owner to improve code and waiting for the result needs investigation new feature breaking changes labels Feb 28, 2023
@panjf2000 panjf2000 added this to the Long term milestone Feb 28, 2023
@someview
Copy link

这个应该不太好做。需要兼容go原生的context

@xsean2020
Copy link

// import "context"
//
// // User is the type of value stored in the Contexts.
// type User struct {...}
//
// // key is an unexported type for keys defined in this package.
// // This prevents collisions with keys defined in other packages.
// type key int
//
// // userKey is the key for user.User values in Contexts. It is
// // unexported; clients use user.NewContext and user.FromContext
// // instead of using this key directly.
// var userKey key
//
// // NewContext returns a new Context that carries value u.
// func NewContext(ctx context.Context, u *User) context.Context {
// return context.WithValue(ctx, userKey, u)
// }
//
// // FromContext returns the User value stored in ctx, if any.
// func FromContext(ctx context.Context) (*User, bool) {
// u, ok := ctx.Value(userKey).(*User)
// return u, ok
// }
估计只能用go 目前推荐的方式

Copy link

This issue is marked as stale because it has been open for 30 days with no activity.

You should take one of the following actions:

  • Manually close this issue if it is no longer relevant
  • Comment if you have more information to share

This issue will be automatically closed in 7 days if no further activity occurs.

@github-actions github-actions bot added the stale label Sep 26, 2024
@panjf2000 panjf2000 removed the stale label Sep 26, 2024
Copy link

This issue is marked as stale because it has been open for 30 days with no activity.

You should take one of the following actions:

  • Manually close this issue if it is no longer relevant
  • Comment if you have more information to share

This issue will be automatically closed in 7 days if no further activity occurs.

@github-actions github-actions bot added the stale label Oct 27, 2024
@panjf2000 panjf2000 removed the stale label Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes enhancement New feature or request needs investigation new feature pending development Requested PR owner to improve code and waiting for the result proposal Proposal for this repo
Projects
None yet
Development

No branches or pull requests

4 participants