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

Follow-up of pull request #74 #77

Open
2 tasks
nunoguedelha opened this issue Nov 10, 2021 · 1 comment
Open
2 tasks

Follow-up of pull request #74 #77

nunoguedelha opened this issue Nov 10, 2021 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@nunoguedelha
Copy link
Collaborator

nunoguedelha commented Nov 10, 2021

It's worth gathering our discussions in a new issue in order to track the investigation. It should have lower priority than the 3 next fixes in the reference list #33 , but it's worth addressing the raised points.

Originally posted by @nunoguedelha in #74 (comment)


@S-Dafarra :
What happens if you put the address of a different machine?

@nunoguedelha :
HaHaaaa! I knew you were gonna ask that! I have no idea. There might be internal mechanisms or shenanigans connecting to a remote machine and launching a server. But I doubt it.

It's something worth checking. At the end, if localhost or the machine's self address are the only left choices, then we should only allow those and use a keyword for the machine's self IP, like... selfIPaddress, and resolve it in the Node.js/Javascript scripts.

What do you think?

@S-Dafarra :

At the end, if localhost or the machine's self address are the only left choices, then we should only allow those and use a keyword for the machine's self IP, like... selfIPaddress, and resolve it in the Node.js/Javascript scripts.

Yeah, I posed the question leaving some room for some fancy mechanism of Node.js I did not know about 😄 . But in general, I think we don't need that feature. If we need to do that, we could resort to ssh. Also resolving automatically the address could be dangerous if you have multiple network cards. Can we check if the address we set is actually the address of the machine, and throw a warning otherwise?

If we leave the possibility to the user to specify the address, it is quite probable he/she will mess up. I simply want to reduce that chance.

@nunoguedelha :

But in general, I think we don't need that feature. If we need to do that, we could resort to ssh

Yes it's safer to stay simple and only consider using the address(es) of the machine.

Can we check if the address we set is actually the address of the machine, and throw a warning otherwise?
If we leave the possibility to the user to specify the address, it is quite probable he/she will mess up. I simply want to reduce that chance.

Agree, I think it's possible, I have to investigate. We get the possible IP addresses and throw a warning if none of them matches the one set by the user.


@S-Dafarra :
Wouldn't it be possible to also get this list directly from the json file?

@nunoguedelha :
It's a good point, actually, I intend to simplify this function as well as the update. But I shouldn't need to save a field telling if the entry is already flat or not. It would be simpler to systematically call flatten and do the check in there. Btw, I think it's already possible to do it without additional checks, I have to take a look.

@S-Dafarra :
I see. My main point would be to simplify the logic for adding new ports. Having a single json file where you specify what you want would be already nice, but clearly this would be for future works

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant