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

added grpc interceptor and singleton changes #130

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

Kaivalya1997
Copy link
Contributor

No description provided.


//If GetClientOpts called before SetClientOpts, it will return a default ClientOpts
func GetClientOpts() *ClientOpts {
once.Do(func() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this cause an issue if set is called before get. The clientOptsSingleton would be nil

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if once.Do(f) is called multiple times, only the first call will invoke f, even if f has a different value in each invocation. A new instance of Once is required for each function to execute.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the expected behaviour is to call Set before Get, which the developer needs to ensure. In case if Set is not called and Get is invoked first, the default value of the NoExitOnConnFailure will be set to false as once.Do will execute once and set it once finally. In case Get is called before, the value will not be nil. As the NoExitOnConnFailure flag is set at the beginning of the mocctl process, I am ensuring that for mocctl the Set is being called before any Get operations occur

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cant we just check if clientOptsSingleton is not nil and return default if nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do that as well, but we would need to take care of thread safety explicitly. I used once.Do because it is inherently thread safe. Do you want me to implement the nil checking logic?

@msftpgayam
Copy link
Contributor

@Kaivalya1997 do we still want to pick up this PR? If not, can we close it?

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

Successfully merging this pull request may close these issues.

3 participants