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

Clnrest: update config param with cln prefix #6857

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

ShahanaFarooqui
Copy link
Collaborator

This will allow users to use clnrest with c-lightning-REST without conflicts.
It was required for applications to have enough time for migrating from c-lightning-REST to clnrest.

Changelog-Changed:
config option rest-certs changed to clnrest-certs
config option rest-protocol changed to clnrest-protocol
config option rest-host changed to clnrest-host
config option rest-port changed to clnrest-port
config option rest-cors-origins changed to clnrest-cors-origins
config option rest-csp changed to clnrest-csp

Copy link
Collaborator

@nepet nepet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All I got are just nits, besides maybe the sleeps in the tests but if they don't introduce more flakes this is fine as well. I really like the level of detail in the tests and how much easier the rpc method call gets using RuneErrors.

ACK 128eaf3

tests/test_clnrest.py Outdated Show resolved Hide resolved
tests/test_clnrest.py Outdated Show resolved Hide resolved
tests/test_clnrest.py Outdated Show resolved Hide resolved
tests/test_clnrest.py Outdated Show resolved Hide resolved
@nepet nepet added this to the v23.11 milestone Nov 10, 2023
poetry.lock Outdated Show resolved Hide resolved
@ShahanaFarooqui
Copy link
Collaborator Author

All I got are just nits, besides maybe the sleeps in the tests but if they don't introduce more flakes this is fine as well. I really like the level of detail in the tests and how much easier the rpc method call gets using RuneErrors.

ACK 128eaf3

Shout out to @tonyaldon for detailed testcases :). Two sleep timers and request retries are necessary for CI to succeed and I removed other timers.

This will allow users to use clnrest with c-lightning-REST without conflicts.
It was required for applications to have enough time for migrating from c-lightning-REST to clnrest.

Changelog-Changed:
config option `rest-certs` changed to `clnrest-certs`
config option `rest-protocol` changed to `clnrest-protocol`
config option `rest-host` changed to `clnrest-host`
config option `rest-port` changed to `clnrest-port`
config option `rest-cors-origins` changed to `clnrest-cors-origins`
config option `rest-csp` changed to `clnrest-csp`
@nepet nepet force-pushed the clnrest-config-params branch from c42efb0 to 70d18f4 Compare November 16, 2023 08:11
@nepet
Copy link
Collaborator

nepet commented Nov 16, 2023

Rebased

@nepet nepet merged commit cc2665d into ElementsProject:master Nov 16, 2023
38 checks passed
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.

2 participants