astrosonic / sanctuary

A secure synchronous lightweight chatroom with zero logging and total transience
GNU General Public License v3.0
54 stars 11 forks source link

Add Option for IP Address #59

Closed amaank404 closed 7 months ago

amaank404 commented 4 years ago

I have seen that this project does not allow to choose IP Address for hosting the Web Application, so i thought I can fix it. Hence I will be getting a new pull request Up for the same.

gridhead commented 4 years ago

Thank you for marking that out. @xcodz-dot.

The community usually goes into replicating the defect/bug that is listed, then ideates the way of solving it and finally moves on to developing a fix to address the said issue. You are requested to discuss in detail under this ticket how such a feature addition can be important and why it should be incorporated.

amaank404 commented 4 years ago

well, when you host a server, you need to host on a static IP address which can be accessed by a DNS Server. Also you might want to host the server on local LAN connection so you might want to put on a custom IP Address for the server to host. So i suggest we can simply add an argument to click and pass the address to run method of flask application

amaank404 commented 3 years ago

Please assign this to me

gridhead commented 3 years ago

This needs some thorough discussions. I'd ask @VaibhavSaini19 @s-katte @shivangswain @tripathyprateek to explore this and get into discussion.

s-katte commented 3 years ago

well, when you host a server, you need to host on a static IP address which can be accessed by a DNS Server. Also you might want to host the server on local LAN connection so you might want to put on a custom IP Address for the server to host. So i suggest we can simply add an argument to click and pass the address to run method of flask application

As I understand, you are suggesting that instead of default localhost:PORT only, we can have and option that user can provide input for IP as well, like suppose 198.6.6.1:PORT ? Please correct me if i am wrong @xcodz-dot

amaank404 commented 3 years ago

Yes, by default we will keep it localhost but if user specifies the option we will keep the user one. This would not break compatibility but only add a optional command line option. Also we will take input in this format 192.168.43.1 and not 192.168.43.1:PORT

shivangswain commented 3 years ago

This is a good feature to have in my opinion and I don't see any problem with merging only this change from the linked PR. I'd suggest @xcodz-dot drop the other additions they have made to the PR and only let this functionality be added to the project for now. (see: Atomic PRs)

amaank404 commented 3 years ago

i did not quite understand, what i am thinking is:

amaank404 commented 3 years ago

please assign this to me :smile:

tripathyprateek commented 3 years ago

please assign this to me 😄

@t0xic0der please assign

amaank404 commented 3 years ago

This is a good feature to have in my opinion and I don't see any problem with merging only this change from the linked PR. I'd suggest @xcodz-dot drop the other additions they have made to the PR and only let this functionality be added to the project for now. (see: Atomic PRs)

I just understood it now, but i think i will startup clean with a new PR

amaank404 commented 3 years ago

@t0xic0der please assign this to me :smile:

shivangswain commented 3 years ago

I apologize for the delay in assigning; you may start contributing now.

amaank404 commented 3 years ago

thanks! 😄

gridhead commented 3 years ago

Hi @xcodz-dot,

I know it has been a long while since we last attended your issues/PRs and I understand that it can be disheartening to be in such a situation. We would definitely get back to working on this at a later date, whenever we would find some time out of the tasks we are usually engaged in. Thanks for contributing. :)

amaank404 commented 3 years ago

:)