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

Do not require https in address fields on the simulation file #154

Open
sr-gi opened this issue Oct 26, 2023 · 4 comments
Open

Do not require https in address fields on the simulation file #154

sr-gi opened this issue Oct 26, 2023 · 4 comments
Assignees
Labels
feature New feature or request good first issue Good for newcomers Low Priority
Milestone

Comments

@sr-gi
Copy link
Member

sr-gi commented Oct 26, 2023

Currently, we force addresses in the simulation file to start with https://. That's not really user-friendly.

Make it so the can be specified but not required (as in, if they are not required we just add it in).

@sr-gi sr-gi added feature New feature or request good first issue Good for newcomers Low Priority labels Oct 26, 2023
@sr-gi
Copy link
Member Author

sr-gi commented Oct 26, 2023

Motivated by #152 (comment)

@okjodom okjodom self-assigned this Nov 6, 2023
@okjodom
Copy link
Collaborator

okjodom commented Nov 6, 2023

I suspect this needs a tonic-lnd update to use http connector. Will look into it :)

@sr-gi
Copy link
Member Author

sr-gi commented Nov 6, 2023

Oh, I was really thinking of something pretty straightforward here, like an if checking the string front to strip the protocol, if it is not https complain, and if it does not have one just add it.

If the only way to use tonic is via https, it may be worth adding the patch upstream, otherwise I wouldn't bother.

@okjodom
Copy link
Collaborator

okjodom commented Nov 6, 2023

Oh, I was really thinking of something pretty straightforward here, like an if checking the string front to strip the protocol, if it is not https complain, and if it does not have one just add it.

Quick UX win for SimLn, yes

If the only way to use tonic is via https, it may be worth adding the patch upstream, otherwise I wouldn't bother.

Yeah the current version of tonic requires https per this comment so I would look at the reasons why.

@carlaKC carlaKC added this to the V3 milestone Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request good first issue Good for newcomers Low Priority
Projects
None yet
Development

No branches or pull requests

3 participants