WIP: accept tracker connections

anthonyraymond / joal

An open source command line RatioMaster with an optional WebUI.

Apache License 2.0

595 stars 64 forks source link

WIP: accept tracker connections #187

Open laur89 opened 1 year ago

laur89 commented 1 year ago

~Note this change is on top of #172 -- merge that beforehand.~ merged

laur89 commented 1 year ago

Ye this is going to need quite a bit of research/know-how. Would you be willing to merge this as-is, considering it's reportedly solved problem to one user. And possibly/hopefully it improves existing logic somewhat.

WZrocks commented 1 year ago

Ye this is going to need quite a bit of research/know-how. Would you be willing to merge this as-is, considering it's reportedly solved problem to one user. And possibly/hopefully it improves existing logic somewhat.

Two users. :-) I have been waiting for this merge, and updated Docker image, since the opening of this PR.

anthonyraymond commented 1 year ago

Hello there, i've not forgotten about you.

But i'm lacking time to review that. I try to give it a look ASAP

WZrocks commented 1 year ago

Do you have any idea if and when you'll have the time to look at this, @anthonyraymond? I don't want to rush you, I'm already very grateful for what you did with this project so far, I'm just looking for an ETA.

anthonyraymond commented 1 year ago

Hello, i'm still unsure if i want to merge this as-is or not. Not sure if that makes its more unsafe or not.

An analogy for this PR is:

before nobody won't ever answer the call when the phone rings.
Now someone will, but he is clearly not speaking the language that you expect the people to speak. You'll know it's not the person it is supposed to be on the other end.

WZrocks commented 1 year ago

That is a valid point. Would it be possible to make it optional in config? @laur89 @anthonyraymond

rursache commented 1 year ago

@anthonyraymond i think it's best to just add a toggle in settings. for me it works great but maybe others can have issues, indeed. can't wait to see this merged and published <3

WZrocks commented 1 year ago

I don't think the config option will come anytime soon either, right? 😔

rursache commented 11 months ago

@anthonyraymond can we please merge this as well? been using it since then and it's all good. maybe add a config toggle in the settings just in case?

anthonyraymond commented 11 months ago

@rursache i don't think this is ever going to be added as is. It's sounds a bit dangerous for most people.

I'll include a real and complete implementation in the next version (built from scratch), but for now i prefer not to include it as is

rursache commented 11 months ago

@anthonyraymond I'll include a real and complete implementation in the next version (built from scratch), but for now i prefer not to include it as is

thank you, looking forward to your implementation!