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

Adds proxy configuration option via environment variables with global agent fixes issue #56 #74

Merged
merged 3 commits into from
Feb 8, 2023

Conversation

rvalle
Copy link

@rvalle rvalle commented Feb 7, 2023

The node-red tibber api modules depends on tibber-api to source the data, and tibber-api depends on ws (web service) to source the data.

Adding proxy support is very easy to achieve by simply importing global-agent and setting the right environment variables,

As the documentation of ws states in order to use a proxy:

Use a custom http.Agent implementation

This is exactly what http-agent does, allowing configuration via environment variables.

How to test the proxy feature

On your development environment deploy your favorite proxy server using docker, such as:

docker run --rm --name squid-container -e TZ=UTC -p 3128:3128 ubuntu/squid

Now you have a proxy running on 127.0.0.1:3128 We will now use it as:

GLOBAL_AGENT_HTTPS_PROXY=http://127.0.0.1:3128 GLOBAL_AGENT_HTTP_PROXY=http://127.0.0.1:3128 node-red

We are asking global-agent to forward http and https requests via this proxy.

A tibber-feed will start working via the proxy. I.e. stop the proxy and watch your feed break.

There are multiple confirmations possible all which can be done via environment variables:

For details check the global agent documentation

Fixes issue #56

@bisand
Copy link
Owner

bisand commented Feb 7, 2023

Thanks for contributing!

It's not my intention to be pessimistic here, but I was hoping to implement the proxy handling without introducing another dependency. That might be far fetched.. I also see that the global-agent haven't been updated for a few years, and that could lead to potential problems down the line. We could maybe consider implementing it on tibber-api instead (issue 21), since the node-red module depends on that?

Let me consider the changes and do some testing, and I will get back to you as soon as I can.

It would be nice to hear your thoughts about using global-agent as a dependency and also maybe moving it to tibber-api?

Thanks!

@bisand bisand merged commit 88ae80e into bisand:master Feb 8, 2023
@bisand
Copy link
Owner

bisand commented Feb 8, 2023

Tested OK. Seems like we can use this approach for now.
@rvalle, thanks again for the contribution!

@rvalle
Copy link
Author

rvalle commented Feb 8, 2023

@bisand, you are welcome.

I agree there could be nicer approaches, but at the end of the day its what the WS package owners decide.

It is possible to programmatically change the prefix of the ENV variables, if you are interested.

I have not researched myself other providers similar to global-agent, but this I have in production on other projects for years.

@rvalle
Copy link
Author

rvalle commented Feb 8, 2023

@bisand I also thought about pushing the request to tibber API, but I thought that Open Source projects would hit production first :)

I can see that it only took you a few hours to get it released.

I will actually open an issue on Tibber API, referencing this issue, in case they take the same approach, then we can remove the dependency.

I agree the dependency does not belong to this project, as it is not dealing with the "wire" of data.

@rvalle
Copy link
Author

rvalle commented Feb 8, 2023

ok @bisand , I did not realize you were the author of tibber-api

In that case perhaps it makes sense to move the dependency there, but that is your call.

@bisand
Copy link
Owner

bisand commented Feb 8, 2023

ok @bisand , I did not realize you were the author of tibber-api

In that case perhaps it makes sense to move the dependency there, but that is your call.

No problem! :)

I have been thinking about that, and I think we can have the dependency as it is for now. tbber-api have a few extra projects that are dependent on it and it will require a bit more testing before I can release a version with proxy support.

Feel free to submit a PR to tibber-api if you have time for that.

Thanks!

@bisand bisand mentioned this pull request Nov 7, 2023
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.

None yet

2 participants