Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

TelemetryEndpoints must be valid MutliAddr URL #5069

Merged

Conversation

pscott
Copy link
Contributor

@pscott pscott commented Feb 26, 2020

Previously, when creating a telemetry endpoint we would not verify the validity of the provided URL. If we the URL was invalid, we would just throw a warning!, saying that the URL was invalid.

With this PR, we move this parsing logic to the creation of the TelemetryEndpoints struct.

@parity-cla-bot
Copy link

It looks like @pscott signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@pscott pscott marked this pull request as ready for review February 26, 2020 17:20
@pscott pscott changed the title Check for url validity when creating TelemetryEndpoints Parse URL when creating TelemetryEndpoints Feb 26, 2020
@pscott pscott added the A0-please_review Pull request needs code review. label Feb 26, 2020
client/telemetry/src/lib.rs Outdated Show resolved Hide resolved
@tomaka tomaka self-requested a review February 26, 2020 18:48
@pscott pscott added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Feb 27, 2020
@tomaka
Copy link
Contributor

tomaka commented Feb 27, 2020

Also this is technically a breaking change, so I guess we would wait for 3.0 to land it?
(cc @gnunicorn)

@pscott
Copy link
Contributor Author

pscott commented Feb 27, 2020

Should we merge this one as well as #5057 into one bigger PR? Might be clearer?

Scott Piriou added 2 commits February 28, 2020 11:47
@pscott
Copy link
Contributor Author

pscott commented Feb 29, 2020

Tests are failing on CI but they succeeded locally on my machine. Last commits that got merged to master were failing CI too.. Not sure what the protocol is in this situation... :)

@pscott pscott requested a review from tomaka March 2, 2020 10:25
@pscott pscott requested a review from NikVolf March 2, 2020 10:25
@pscott pscott added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Mar 17, 2020
@gnunicorn gnunicorn changed the title Parse URL when creating TelemetryEndpoints Parse URL into MutliAddr when creating TelemetryEndpoints Mar 25, 2020
@gnunicorn gnunicorn changed the title Parse URL into MutliAddr when creating TelemetryEndpoints TelemetryEndpoints must be valid MutliAddr URL Mar 25, 2020
client/telemetry/src/lib.rs Show resolved Hide resolved
@gnunicorn gnunicorn added this to the 2.0 milestone Mar 25, 2020
@gnunicorn gnunicorn added A6-mustntgrumble and removed A0-please_review Pull request needs code review. labels Mar 25, 2020
bin/node/cli/src/chain_spec.rs Outdated Show resolved Hide resolved
client/telemetry/src/lib.rs Outdated Show resolved Hide resolved
@gnunicorn gnunicorn merged commit fe23d88 into paritytech:master Mar 26, 2020
@gui1117
Copy link
Contributor

gui1117 commented Mar 26, 2020

polkadot PR paritytech/polkadot#941

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Creating a TelemetryEndpoints should parse the URL
7 participants