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: context.Context Support #100

Open
2 tasks done
VoyTechnology opened this issue Sep 19, 2023 · 6 comments · May be fixed by #121
Open
2 tasks done

Feature: context.Context Support #100

VoyTechnology opened this issue Sep 19, 2023 · 6 comments · May be fixed by #121
Labels
enhancement New feature or request

Comments

@VoyTechnology
Copy link

VoyTechnology commented Sep 19, 2023

Is your feature request related to a problem?

This would be a part of a wider change to allow trace information propagation (if possible). Passing in the context is currently not possible and we would need it to propagate the trace information. Seeing how the Surreal server already supports tracing, this would provide a very good method of troubleshooting entire requests.

Describe the solution

The standard way to do so is by using context.Context and passing it as the first argument to the function. eg.

func (db *DB) Create(ctx context.Context, thing string, data any) (any, error)

This would be the preferred way as the SDK has not yet reached v1.0 so it's easier to change the method signature without a v2 release.

Alternative methods

If the method signatures are not to be changed, the Context suffix can be added to each method. This is rather verbose to type and is typically used when a pre-existing API is adding context support.

eg.

func (db *DB) CreateContext(ctx context.Context, thing string, data any) (any, error)

There is no other obvious way to add context information.

SurrealDB version

N/A

Contact Details

No response

Is there an existing issue for this?

  • I have searched the existing issues

Code of Conduct

  • I agree to follow this project's Code of Conduct
@partik03
Copy link

@VoyTechnology I just wanted to ask a question.Currently the demand of this issue is just to make the existing functions support context or should we have to change functions so that every functions just needs the context parameter and all the data to be used in the function body will be extracted from the context body itself?

@VoyTechnology
Copy link
Author

@partik03 I am not sure I fully understand your question so I will try and answer based on my interpretation:

We would pass in a new context.Context argument at the beginning of every function. thing string, data any will remain unmodified. How the context will be used after being passed into the function is outside the scope of this feature request, but it is safe to assume it can be used to handle tracing, request cancellation, log context etc., but the actual data that will be sent will still be part of the data any parameter.

@partik03
Copy link

@VoyTechnology got it.Thanks!

@partik03
Copy link

@VoyTechnology can I take up this one?

@partik03
Copy link

@VoyTechnology As the project uses gorilla for making websocket connections and gorilla does not have a single context variable as you mentioned above so should I use the standard http request and response variables or should I include a library like gofiber which takes care of the context in a single variable?

@phughk
Copy link
Contributor

phughk commented Dec 1, 2023

Hey @VoyTechnology @partik03 !

We do intend to standardise the interface, which would include the addition of context.Context.

If you @partik03 or someone else would like to add it to the existing API then we would definitely merge that. But just giving a heads up that it is likely to change in the future.

@phughk phughk added enhancement New feature or request and removed Hacktoberfest labels Dec 1, 2023
@VoyTechnology VoyTechnology linked a pull request Jan 9, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants