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

Definition of host parameter #19

Open
MarcusZuber opened this issue Feb 14, 2023 · 4 comments
Open

Definition of host parameter #19

MarcusZuber opened this issue Feb 14, 2023 · 4 comments
Assignees

Comments

@MarcusZuber
Copy link
Member

We have multiple ways to define the host-parameter: the param-default value, the class-paramters, passed to the plugin-manager and the environment variable.

In the current implementation, the parmeters from the plugin-manager are completly ignored (which is not good. currently I'm fiddeling around with environment variables in concert, since I can not define it in the constructor).
Currently https://github.com/ufo-kit/uca-net/blob/master/uca-net-camera.c#L618 reads the environment variable and depending on its exitence sets the host property to its value or to "localhost".

Now the question: Which configuration should be stronger?
I would suggest that a set paramter via the plugin-manager should not be overwritten from an environment-variable. If it isnot set via the manager we check the environment variable and if both are not set we set it to localhost.

Why should we set it to localhost as default? The only use-case is for running local tests.

@MarcusZuber MarcusZuber self-assigned this Feb 14, 2023
@MarcusZuber MarcusZuber changed the title Defintion of host parameter Definition of host parameter Feb 14, 2023
@MarcusZuber
Copy link
Member Author

When you run the example from the readme

from concert.devices.cameras.uca import Camera
camera = Camera('net', {'host': 'foo.bar:1234'})

you will get an "can not connect to 'localhost' error.

@matze
Copy link
Contributor

matze commented Feb 14, 2023

Of course I do not have any final say into that but to me, environment variables usually override any build time configuration otherwise they are pretty useless. But not taking the property value is indeed stupid.

@tfarago
Copy link
Contributor

tfarago commented Feb 15, 2023

So as I understand Marcus will take care of this?

@MarcusZuber
Copy link
Member Author

Sure. I will come up with a PR today or tomorrow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants