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.

Sign up for GitHub

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

Jump to bottom

Use JSON instances for NodeToNodeVersion & NodeToClientVersion #4691

Merged
merged 6 commits into from
Mar 26, 2023
Merged

Use JSON instances for NodeToNodeVersion & NodeToClientVersion #4691

merged 6 commits into from
Mar 26, 2023

Conversation

coot
Copy link
Contributor

@coot coot commented Dec 2, 2022

  • NodeToNodeVersion and NodeToClientVersion JSON instances
  • Split WarningDevelopmentNetworkProtocolVersions

@coot coot added the networking Issues and PRs related to networking label Dec 2, 2022
@coot
Copy link
Contributor Author

coot commented Dec 2, 2022

This is a small improvement for #4630.

@coot coot self-assigned this Dec 2, 2022
@coot coot force-pushed the coot/json-network-version-repr branch from 056867e to c477057 Compare December 12, 2022 14:19
Jimbo4350
Jimbo4350 previously requested changes Jan 10, 2023
View reviewed changes
cardano-node/src/Cardano/Node/Tracing/Tracers/Startup.hs Outdated Show resolved Hide resolved
cardano-node/src/Cardano/Tracing/OrphanInstances/Network.hs Outdated Show resolved Hide resolved
cardano-node/src/Cardano/Tracing/OrphanInstances/Network.hs Outdated Show resolved Hide resolved
cardano-node/src/Cardano/Node/Run.hs Show resolved Hide resolved
@coot coot force-pushed the coot/json-network-version-repr branch from c477057 to 489d871 Compare January 19, 2023 18:31
@coot coot requested a review from Jimbo4350 January 19, 2023 18:31
@coot coot force-pushed the coot/json-network-version-repr branch 3 times, most recently from 15fab2c to bfd3bde Compare January 20, 2023 17:14
@coot coot force-pushed the coot/json-network-version-repr branch from bfd3bde to 490fd94 Compare March 8, 2023 08:25
@coot coot requested a review from a team as a code owner March 8, 2023 08:25
@coot
Copy link
Contributor Author

coot commented Mar 8, 2023

I cherry picked some more commits from release/1.35 branch which didn't make it to the master branch.

@coot coot dismissed Jimbo4350’s stale review March 9, 2023 10:08

The review comments were addressed.

@coot coot force-pushed the coot/json-network-version-repr branch 3 times, most recently from 298ccb9 to 4331fc5 Compare March 13, 2023 10:11
@coot coot force-pushed the coot/json-network-version-repr branch from 4331fc5 to 30b2c04 Compare March 13, 2023 14:21
Jimbo4350
Jimbo4350 reviewed Mar 17, 2023
View reviewed changes
cardano-node/src/Cardano/Node/Tracing/StateRep.hs Show resolved Hide resolved
cardano-node/src/Cardano/Node/Tracing/StateRep.hs Show resolved Hide resolved
cardano-node/src/Cardano/Node/Tracing/Tracers/Startup.hs Show resolved Hide resolved
cardano-node/src/Cardano/Tracing/OrphanInstances/Network.hs Outdated Show resolved Hide resolved
cardano-node/src/Cardano/Tracing/OrphanInstances/Network.hs Outdated Show resolved Hide resolved
cardano-node/src/Cardano/Node/Startup.hs
-- | Warn that peer-to-peer requires
-- 'TestEnableDevelopmentNetworkProtocols' to be set.
--
| P2PWarningDevelopementNetworkProtocols
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean we no longer need the TestEnableDevelopmentNetworkProtocols flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't, but we might need it again in the future, so I think it's ok to keep it. There's no difference (for the user) if we parse it and not use it or not parse it at all.

@coot coot force-pushed the coot/json-network-version-repr branch 2 times, most recently from 6c94f79 to 125c8f8 Compare March 20, 2023 14:56
@Jimbo4350 Jimbo4350 requested review from Jimbo4350 and removed request for LudvikGalois March 24, 2023 13:27
@coot
Copy link
Contributor Author

coot commented Mar 24, 2023

bors merge

iohk-bors bot added a commit that referenced this pull request Mar 24, 2023
@iohk-bors @coot
Merge #4691
aa76134 
4691: Use JSON instances for NodeToNodeVersion & NodeToClientVersion r=coot a=coot

- NodeToNodeVersion and NodeToClientVersion JSON instances
- Split WarningDevelopmentNetworkProtocolVersions


Co-authored-by: Marcin Szamotulski <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Mar 24, 2023

This PR was included in a batch that successfully built, but then failed to merge into master. It will not be retried.

Additional information:

{"message":"Validation Failed","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@coot @newhoggy
Split WarningDevelopmentNetworkProtocolVersions
6e3dfcb 
Split it into node-to-node and node-to-client. This is needed so that the
log contains information whether the version numbers (now encoded as
numbers) are node-to-node or node-to-client version.
@coot @newhoggy
NodeToNodeVersion and NodeToClientVersion JSON instances
26032df
@coot @newhoggy
Reword where to get peers from in understanding-config-files.md
b73b59d 
Fixes input-output-hk/ouroboros-network#3691
@coot @newhoggy
Removed P2PWarningDevelopmentNetworkProtocols
f6bc4f2 
The warning is not needed as of ouroboros-network#3859.
@coot @newhoggy
Modify P2PWarning message
e15044b 
P2P is now an early release: we encourage to enable it on at most one of
the relays (if one has more than one relay).  We don't advise yet to use
it on BP nodes.
@coot @newhoggy
Picked changelog from 1.35.6 release
16805e6
@newhoggy newhoggy force-pushed the coot/json-network-version-repr branch from cf3ecc1 to 16805e6 Compare March 26, 2023 08:17
@coot coot enabled auto-merge March 26, 2023 08:25
@coot coot added this pull request to the merge queue Mar 26, 2023
Merged via the queue into master with commit 0592629 Mar 26, 2023
@iohk-bors iohk-bors bot deleted the coot/json-network-version-repr branch March 26, 2023 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dnadales dnadales dnadales left review comments

@Jimbo4350 Jimbo4350 Jimbo4350 approved these changes

@newhoggy newhoggy Awaiting requested review from newhoggy

@deepfire deepfire Awaiting requested review from deepfire

@jutaro jutaro Awaiting requested review from jutaro

@dcoutts dcoutts Awaiting requested review from dcoutts

@erikd erikd Awaiting requested review from erikd

@JaredCorduan JaredCorduan Awaiting requested review from JaredCorduan

@CarlosLopezDeLara CarlosLopezDeLara Awaiting requested review from CarlosLopezDeLara

Assignees

@coot coot

Labels
area: trace networking Issues and PRs related to networking
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@coot @dnadales @Jimbo4350