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

Support for context.Context in func(any, []any) any #218

Open
AuroraV opened this issue Jul 2, 2023 · 5 comments
Open

Support for context.Context in func(any, []any) any #218

AuroraV opened this issue Jul 2, 2023 · 5 comments

Comments

@AuroraV
Copy link

AuroraV commented Jul 2, 2023

  I am very grateful for the excellent open-source code you have provided, and I have used it in my project. However, I noticed that your func(any, []any) any function does not support the context.Context parameter. I saw the env.ctx parameter in your source code, so I was wondering if it would be possible to update the function signature to support the context.Context parameter. This would greatly enhance the usability of your code and make it more compatible with other libraries.
Here is an example of what I saw in your code:

x, args := env.pop(), env.args[:argcnt]
for i := 0; i < argcnt; i++ {
    args[i] = env.pop()
}
w := v[0].(func(any, []any) any)(x, args)

  I noticed that the env.ctx variable is consistent with the context.Context type. Therefore, I think adding the context.Context parameter would be a great improvement.

  If you have any suggestions or recommended supported methods, please let me know. Thank you very much for your time and help.

Best regards.

@itchyny
Copy link
Owner

itchyny commented Jul 4, 2023

I'm just for curious, what kind of custom function you want to integrate with gojq?

@AuroraV
Copy link
Author

AuroraV commented Sep 7, 2023

I'm just for curious, what kind of custom function you want to integrate with gojq?

@itchyny Sorry for such late response.I need to follow up on the jq function debug results obtained by inputting different http bodies for the same jq code, and share the same session information under the same request. eg:

	optFn := func(key string) gojq.CompilerOption {
		return gojq.WithFunction(key, 1, 1,
			func(ctx context.Context, a any, _ []any) any {
				claim := getClaim(ctx)
				id, _ := a.(int64)
				name, _ := callHttp(claim.sessinId, id)
				claim.DebugMap[key] = name
				return name
			})
	}

	userInputCode := `{"user_name":getUserName(.user_id), "company_name":getCompanyName(.company_id)}`
	query, _ := gojq.Parse(userInputCode)
	code, _ := gojq.Compile(query, optFn("getUserName"), optFn("getCompanyName"))

	Handler := func(w http.ResponseWriter, r *http.Request) {
		// ctx has some claims from middleware
		ctx := r.Context()
		body, _ := ioutil.ReadAll(r.Body)
		var data map[string]interface{}
		_ = json.Unmarshal(body, &data)
		res := code.RunWithContext(ctx, body)
		val, _ := res.Next()
		if err, ok := val.(error); ok {
			panic(err)
		}
		result := val.(map[string]interface{})
		if err := doSth(result); err != nil {
			claim := getClaim(ctx)
			w.Write(claim.DebugMap)
			w.WriteHeader(http.StatusInternalServerError)
			return
		}
		w.Write([]byte("success"))
		w.WriteHeader(http.StatusOK)
		return
	}

	http.ListenAndServe(":8080", http.HandlerFunc(Handler))

In my case, I implemented a configuration similar to low-code through gojq code (userInputCode). HTTP requests may take message events, and I used WithFunction to provide plugin capabilities to users, allowing them to freely assemble data and do certain things

@xakepp35
Copy link

xakepp35 commented Oct 9, 2023

We also need such functionality, because it turns out there is no way to pass different parameters after code was compiled. Variables and functions can only be set at compile time and nothing passes request context at runtime, and for intense use it looks like we are spending noticeable cpu time inside compile, with lots of memory allocations. It would be supereme if compiled code may be reused, ctx will just do the job.

Not to break compatibility you may, for example, just add opcall_ctx opcode alongside with WithCtxFunction - that way it won't affect neither compatibility, neither execution speed of old code..

Very demanding feature!

@itchyny
Copy link
Owner

itchyny commented Oct 10, 2023

For your usecase you can use WithVariables.

@xakepp35
Copy link

xakepp35 commented Oct 16, 2023

Unfortunately, no, we cant. Its just not enough for all things we're performing with it

  • We have some functions that need to work with the request context.
  • We also have the concepts of "alternate data streams" where we have not only input objects but also associated metadata with them. And we have special functions for working with those. It relies on context from now.
  • We have some jq functions that make network requests by themselves.
  • Ultimately, calling compile is just too slow for our workload.

By adding the ability to work with context, we easily solved all of these problems, so we forked and started using context extensively. We achieved code simplifications and measured 10-30% performance gains with it, which is important for us.

We hope that one day we can drop our temporary fork (its just merging main+json_numbers+AuroraV's work+fixes because his merge request broke something and there were panicking builtins), and switch back to this repository.

All that is required for that is the ability to register a function signature with context, which would accept context from RunWithContext. I even suggested how to do so without breaking changes.

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

No branches or pull requests

3 participants