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

fix: Add missing logs at startup #2391

Conversation

fredcarle
Copy link
Collaborator

Relevant issue(s)

Resolves #2308
Resolves #2309

Description

This PR re-adds startup logs showing the P2P addresses and HTTP endpoint of the node.

Tasks

  • I made sure the code is well commented, particularly hard-to-understand areas.
  • I made sure the repository-held documentation is changed accordingly.
  • I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
  • I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

manually tested in the CLI

Specify the platform(s) on which this was tested:

  • MacOS

@fredcarle fredcarle added bug Something isn't working area/cli Related to the CLI binary labels Mar 7, 2024
@fredcarle fredcarle added this to the DefraDB v0.10 milestone Mar 7, 2024
@fredcarle fredcarle requested a review from a team March 7, 2024 22:37
@fredcarle fredcarle self-assigned this Mar 7, 2024
Copy link

codecov bot commented Mar 7, 2024

Codecov Report

Attention: Patch coverage is 77.77778% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 74.95%. Comparing base (b8d9803) to head (f4bec70).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2391      +/-   ##
===========================================
- Coverage    74.99%   74.95%   -0.04%     
===========================================
  Files          268      268              
  Lines        26003    26017      +14     
===========================================
  Hits         19500    19500              
- Misses        5180     5190      +10     
- Partials      1323     1327       +4     
Flag Coverage Δ
all-tests 74.95% <77.78%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
net/peer.go 80.00% <100.00%> (+0.11%) ⬆️
http/server.go 96.74% <90.00%> (-3.26%) ⬇️
node/node.go 67.33% <33.33%> (-4.55%) ⬇️

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b8d9803...f4bec70. Read the comment docs.

Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

LGTM, assuming you fix the lint error before merge :)

@fredcarle fredcarle force-pushed the fredcarle/fix/missing-logs-at-startup branch from 40969e1 to f4bec70 Compare March 8, 2024 00:15
Copy link
Member

@shahzadlone shahzadlone left a comment

Choose a reason for hiding this comment

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

Cheers

@fredcarle fredcarle merged commit 56da65b into sourcenetwork:develop Mar 8, 2024
31 of 32 checks passed
@fredcarle fredcarle deleted the fredcarle/fix/missing-logs-at-startup branch March 8, 2024 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cli Related to the CLI binary bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Listener error logged on server close Missing info message on start
3 participants