You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/11/06 19:15:51 UTC

[GitHub] [pulsar] flowchartsman opened a new pull request #8467: Pf go startup config

flowchartsman opened a new pull request #8467:
URL: https://github.com/apache/pulsar/pull/8467


   ### Motivation
   The pulsar go functions SDK has no access to user config outside of the function context, which is only accessible from inside the function handler itself (i.e. once processing has started). This is a somewhat awkward access method when it comes to user configuration values which might be used to initialize a persistent connection to a database or other service, since it would require a largely-superfluous check for the initialization and related configuration values in every run of the function, when it's only required once at startup.
   ### Modifications
   
   Two global methods have been provided to access the user config map and individual values from it, by key.
   
   ### Verifying this change
   This change is predicated on the changes (#8132)  made towards solving #8216, and will need to be tested in conjunction with them, however a very similar example should suffice:
   
   ```go
   package main
   
   import (
   	"context"
   	"fmt"
   
   	log "github.com/apache/pulsar/pulsar-function-go/logutil"
   	"github.com/apache/pulsar/pulsar-function-go/pf"
   )
   
   func contextFunc(ctx context.Context) {
   	if fc, ok := pf.FromContext(ctx); ok {
   		log.Infof("function ID is:%s, ", fc.GetFuncID())
   		log.Infof("function version is:%s\n", fc.GetFuncVersion())
   		for k := range fc.GetUserConfMap() {
   			log.Infof("Config %q -> \"%v\"", k, fc.GetUserConfValue(k))
   		}
   	}
   }
   
   func main() {
   	conf := pf.GetUserConfig()
   	for k := range conf {
   	        fmt.Printf("Config %q -> \"%v\"\n", k, fc.GetUserConfValue(k))
   	}
   	pf.Start(contextFunc)
   }
   ```
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If `yes` was chosen, please highlight the changes*
   
     - Dependencies (does it add or upgrade a dependency): (yes / no)
     - The public API: (yes)
       - Two new global methods are added to the Pulsar Functions for Go SDK, `GetUserConfig()` and `GetUserConfigValue(key string)`
     - The schema: (no)
     - The default values of configurations: (no)
     - The wire protocol: (no)
     - The rest endpoints: (no)
     - The admin cli options: (no)
     - Anything that affects deployment: (don't know)
   
   #### Public API
   This adds two new global methods to the Pulsar Functions for Go SDK, mentioned above
   ### Documentation
   
     - Does this pull request introduce a new feature? (yes)
     - If yes, how is the feature documented? (not applicable)
     - If a feature is not applicable for documentation, explain why?
       - godoc will suffice


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] flowchartsman commented on pull request #8467: [Go functions] access to user config before function loop

Posted by GitBox <gi...@apache.org>.
flowchartsman commented on pull request #8467:
URL: https://github.com/apache/pulsar/pull/8467#issuecomment-724202429


   Documented `GetMaxIdleTime`


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] flowchartsman commented on pull request #8467: [Go functions] access to user config before function loop

Posted by GitBox <gi...@apache.org>.
flowchartsman commented on pull request #8467:
URL: https://github.com/apache/pulsar/pull/8467#issuecomment-723254042


   I've also added some documentation to the function context method. The only one I ommitted was `GetMaxIdleTime`, since I wasn't sure precisely what this value meant in this context. Please let me know what should go there, and I'll add it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] flowchartsman edited a comment on pull request #8467: [Go functions] access to user config before function loop

Posted by GitBox <gi...@apache.org>.
flowchartsman edited a comment on pull request #8467:
URL: https://github.com/apache/pulsar/pull/8467#issuecomment-723254042


   I've also added some documentation to the function context methods to document the similar functionality. Once I'd done that, it only made sense to satisfy the lint gods and continue. The only one I omitted was `GetMaxIdleTime`, since I wasn't sure precisely what this value meant in this context. Please let me know what should go there, and I'll add it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] freeznet edited a comment on pull request #8467: [Go functions] access to user config before function loop

Posted by GitBox <gi...@apache.org>.
freeznet edited a comment on pull request #8467:
URL: https://github.com/apache/pulsar/pull/8467#issuecomment-725931779


   @wolfstudy @flowchartsman
   I think to get `userConfigs` by create a new `FunctionContext` is a little bit odd, especially in some scenarios like initialize database connections before go function starts. The reason I think it is odd is that `FunctionContext` is an implementation of request-scoped values that passed with `context.Value`, and the context should only be integrated within the instance of go function. Once we propagate `FunctionContext` outside the go function instance, it will break the design of `context`.
   
   So i suggest we have two ways to get `userConfigs`:
   
   - When go function starts (`pf.Start` called), user can get `userConfigs` with `FromContext(ctx)`, like `contextFunc` defined above. This is just like Java and Python functions and follows the design of context.
   - Before go function starts (before `pf.Start` called), user can get `userConfigs` from `Conf` defined in `conf/conf.go`, with json string -> map. 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] flowchartsman commented on pull request #8467: [Go functions] access to user config before function loop

Posted by GitBox <gi...@apache.org>.
flowchartsman commented on pull request #8467:
URL: https://github.com/apache/pulsar/pull/8467#issuecomment-728355187


   > @wolfstudy @flowchartsman
   > I think to get `userConfigs` by create a new `FunctionContext` is a little bit odd, especially in some scenarios like initialize database connections before go function starts. The reason I think it is odd is that `FunctionContext` is an implementation of request-scoped values that passed with `context.Value`, and the context should only be integrated within the instance of go function. Once we propagate `FunctionContext` outside the go function instance, it will break the design of `context`.
   > 
   > So i suggest we have two ways to get `userConfigs`:
   > 
   > * When go function starts (`pf.Start` called), user can get `userConfigs` with `FromContext(ctx)`, like `contextFunc` defined above. This is just like Java and Python functions and follows the design of context.
   > * Before go function starts (before `pf.Start` called), user can get `userConfigs` from `Conf` defined in `conf/conf.go`, with json string -> map.
   
   This method was chosen just for convenience. In point of fact the context is created anew with invocation of `pf.FromContext()` by the unexported functions like so: 
   
   ```go
   instanceConf := newInstanceConf()
   userConfigs := buildUserConfig(instanceConf.funcDetails.GetUserConfig())
   ``` 
   
   So this was the quickest route, but I can issue a PR to do it more directly.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] freeznet commented on pull request #8467: [Go functions] access to user config before function loop

Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #8467:
URL: https://github.com/apache/pulsar/pull/8467#issuecomment-725931779


   I think to get `userConfigs` by create a new `FunctionContext` is a little bit odd, especially in some scenarios like initialize database connections before go function starts. The reason I think it is odd is that `FunctionContext` is an implementation of request-scoped values that passed with `context.Value`, and the context should only be integrated within the instance of go function. Once we propagate `FunctionContext` outside the go function instance, it will break the design of `context`.
   
   So i suggest we have two ways to get `userConfigs`:
   
   - When go function starts (`pf.Start` called), user can get `userConfigs` with `FromContext(ctx)`, like `contextFunc` defined above. This is just like Java and Python functions and follows the design of context.
   - Before go function starts (before `pf.Start` called), user can get `userConfigs` from `Conf` defined in `conf/conf.go`, with json string -> map. 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] wolfstudy commented on pull request #8467: [Go functions] access to user config before function loop

Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #8467:
URL: https://github.com/apache/pulsar/pull/8467#issuecomment-723380566


   >  The only one I omitted was GetMaxIdleTime, since I wasn't sure precisely what this value meant in this context.
   
   GetMaxIdleTime is used for health check between Go Function and Function Worker. GetMaxIdleTime means that after the Go Function exceeds MaxIdleTime, the Function Worker will determine that the function's health check fails.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui merged pull request #8467: [Go functions] access to user config before function loop

Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #8467:
URL: https://github.com/apache/pulsar/pull/8467


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] wolfstudy commented on pull request #8467: [Go functions] access to user config before function loop

Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #8467:
URL: https://github.com/apache/pulsar/pull/8467#issuecomment-725318076


   @flowchartsman Can you help to check the code style and make sure they can pass?
   
   ```
   pf/context.go:30: line is 244 characters (lll)
   // FunctionContext provides contextual information to the executing function. Features like which message id we are handling, whats the topic name of the message, what are our operating constraints, etc can be accessed by the executing function
   pf/context.go:73: line is 127 characters (lll)
   //GetTenantAndNamespace returns the tenant and namespace the pulsar function belongs to in the format of `<tenant>/<namespace>`
   pf/context.go:78: line is 131 characters (lll)
   //GetTenantAndNamespaceAndName returns the full name of the pulsar function in the format of `<tenant>/<namespace>/<function name>`
   pf/context.go:123: line is 149 characters (lll)
   //GetMaxIdleTime returns the amount of time the pulsar function has to respond to the most recent health check before it is considered to be failing.
   pf/function.go:178: line is 122 characters (lll)
   // GetUserConfMap provides a means to access the pulsar function's user config map before initializing the pulsar function
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] wolfstudy commented on pull request #8467: [Go functions] access to user config before function loop

Posted by GitBox <gi...@apache.org>.
wolfstudy commented on pull request #8467:
URL: https://github.com/apache/pulsar/pull/8467#issuecomment-725892264


   /pulsarbot run-failure-checks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org