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

Add a channel_defaults configuration #97

Merged
merged 1 commit into from
Feb 19, 2022

Conversation

allenporter
Copy link
Collaborator

Add a channel_defaults configuration option which is merged with the channel configuration to allow global configuration options like insecure_skip_verify to be specified at startup time, though is flexible to support other options too in the future.

Issue #82

Add a channel_defaults configuration option which is merged with the
channel configuration to allow global configuration options like
insecure_skip_verify to be specified at startup time.
@deepch
Copy link
Owner

deepch commented Feb 19, 2022

But why didn't you use a simple server structure?

"server": {
    "debug": true,
    "http_debug": false,
    "http_demo": true,
    "channel_defaults": {
     "on_demand": true,
   }
//ServerGetCDOnDaemand read debug options
func (obj *StorageST) ServerGetCDOnDaemand() bool {
	obj.mutex.RLock()
	defer obj.mutex.RUnlock()
	return obj.Server.CD.OnDemand
}

I'm not sure if the library should be used here github.com/imdario/mergo

Let's think about how to do without it sorry I think this might add to the problem.

@allenporter
Copy link
Collaborator Author

But why didn't you use a simple server structure? "server": { "debug": true, "http_debug": false, "http_demo": true, "channel_defaults": { "on_demand": true, }

//ServerGetCDOnDaemand read debug options func (obj *StorageST) ServerGetCDOnDaemand() bool { obj.mutex.RLock() defer obj.mutex.RUnlock() return obj.Server.CD.OnDemand }

I'm not sure if the library should be used here github.com/imdario/mergo

Let's think about how to do without it sorry I think this might add to the problem.

My initial thought was just that any option could be global without extra work, however, it is more 'magic' and potentially complex. No problem, I can:

  • Move the options under server
  • Only support limited set of options, without merging

@deepch
Copy link
Owner

deepch commented Feb 19, 2022

maybe

	OnDemand           bool   `json:"on_demand,omitempty" groups:"api,config"`

change to

	OnDemand           *bool   `json:"on_demand,omitempty" groups:"api,config"`

i4.clients = make(map[string]ClientST)

  1. Add function initDefaults()
 func (obj *StorageST)  loadDefaults()  (*ChannelST, error)  {

//create defaults

if obj.OnDemand  == nil {
//use defaults
}else{
//use OnDemand = *obj.OnDemand 
}

}
 

@deepch
Copy link
Owner

deepch commented Feb 19, 2022

I understood your idea, and it is good, but I would consider doing it without using external libraries, taking only part of their functions.

@deepch
Copy link
Owner

deepch commented Feb 19, 2022

there is another idea.

	Defaults           ChannelST    `json:"defaults,omitempty" groups:"api,config"`
	//default audio == true
	//channel audio == false (not set)
	def, _ := json.Marshal(tmp.Server.Defaults)
	for i, i2 := range tmp.Streams {
		for i3, i4 := range i2.Channels {
			log.Println(i4.Audio)
			//result false
			json.Unmarshal(def, &i4)
			log.Fatalln(i4.Audio)
			//result true

....

if the parameters were pointers it would work just fine but right now it replaces.

@deepch
Copy link
Owner

deepch commented Feb 19, 2022

Perhaps you are still right and this option is completely acceptable. thx you

@deepch deepch merged commit 88bb8cc into deepch:master Feb 19, 2022
@deepch
Copy link
Owner

deepch commented Feb 19, 2022

Can we create a chat in telegram or skype?

@allenporter
Copy link
Collaborator Author

allenporter commented Feb 19, 2022

Can we create a chat in telegram or skype?

How about discord? synack#5399

@allenporter allenporter deleted the insecure-skip-verify branch February 21, 2022 20:25
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.

2 participants