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

feat: get peering details #1805

Merged
merged 1 commit into from
Sep 1, 2023
Merged

feat: get peering details #1805

merged 1 commit into from
Sep 1, 2023

Conversation

mkurapov
Copy link
Contributor

@mkurapov mkurapov commented Aug 30, 2023

Changes proposed in this pull request

  • Adding auto peering server (default port 3005)
  • Adding auto peering service & routes that return ilpAddress as well as list of supported assets

Context

Start of auto-peering story. Adding GET route for returning peering details (for peering discoverability). The new server instantiation itself is under a flag (defaults OFF)

Fixes #1763

Checklist

  • Related issues linked using fixes #number
  • Tests added/updated
  • Documentation added
  • Make sure that all checks pass
  • Postman collection updated

@netlify
Copy link

netlify bot commented Aug 30, 2023

Deploy Preview for brilliant-pasca-3e80ec canceled.

Name Link
🔨 Latest commit 6f0caab
🔍 Latest deploy log https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/64ef3bcb1924190008c04f7a

@github-actions github-actions bot added type: tests Testing related pkg: backend Changes in the backend package. type: source Changes business logic labels Aug 30, 2023
@mkurapov mkurapov marked this pull request as ready for review August 31, 2023 10:45
Copy link
Member

@raducristianpopa raducristianpopa left a comment

Choose a reason for hiding this comment

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

LGTM. Just a quick question for my understanding:

Comment on lines 500 to 501
public async shutdown(): Promise<void> {
return new Promise((resolve): void => {
Copy link
Member

Choose a reason for hiding this comment

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

I'm a little bit confused by this. Why is the promise getting resolved multiple times instead of just once? If there is an OP server, wouldn't the promise be resolved in that specific if block, making any further resolves irrelevant?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been trying to update this code, but it's behaving weirdly when I update it to be "correct". I wonder why it was built this way originally, tbh.

I'm going to make a separate PR that hopefully fixes this and makes it more clear

@mkurapov mkurapov merged commit 3250663 into main Sep 1, 2023
@mkurapov mkurapov deleted the mk/1763/get-peer-details branch September 1, 2023 13:39
@mkurapov mkurapov mentioned this pull request Sep 6, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: backend Changes in the backend package. type: source Changes business logic type: tests Testing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add GET /peer discoverability route
2 participants