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

Companion for: try-runtime::follow-chain - keep connection #5968

Merged
paritytech-processbot merged 10 commits into from
Sep 6, 2022
Merged

Companion for: try-runtime::follow-chain - keep connection #5968

Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3bd3513
Adjust to the new API
pmikolajczyk41 Sep 5, 2022
185de69
Remove mutability
pmikolajczyk41 Sep 5, 2022
83a3b21
Async constructor
pmikolajczyk41 Sep 5, 2022
e39b01e
unwrap result
pmikolajczyk41 Sep 5, 2022
5f6fc89
do not panic
pmikolajczyk41 Sep 5, 2022
5059c58
suggestion
pmikolajczyk41 Sep 5, 2022
81e60b9
Merge remote-tracking branch 'origin/master' into piomiko/try-runtime…
pmikolajczyk41 Sep 5, 2022
52dc6e5
leftover
pmikolajczyk41 Sep 5, 2022
75295b0
Merge remote-tracking branch 'origin/master' into piomiko/try-runtime…
Sep 6, 2022
e6f9f5a
update lockfile for {"substrate"}
Sep 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions tests/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// along with Substrate. If not, see <http://www.gnu.org/licenses/>.

use polkadot_core_primitives::Block;
use remote_externalities::rpc_api::get_finalized_head;
use remote_externalities::rpc_api::RpcService;
use std::{
io::{BufRead, BufReader, Read},
process::{Child, ExitStatus},
Expand Down Expand Up @@ -54,9 +54,10 @@ pub async fn wait_n_finalized_blocks(
async fn wait_n_finalized_blocks_from(n: usize, url: &str) {
let mut built_blocks = std::collections::HashSet::new();
let mut interval = tokio::time::interval(Duration::from_secs(6));
let rpc_service = RpcService::new(url, false).await.unwrap();
Copy link
Contributor

@dmitry-markin dmitry-markin Sep 5, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This .unwrap() looks dangerous. Previously such errors were handled by if let below: can't we do lazy initialization in the loop to try to recover from them? CC @niklasad1

Copy link
Contributor Author

@pmikolajczyk41 pmikolajczyk41 Sep 5, 2022

Choose a reason for hiding this comment

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

initially, I put unwrap here since afaik this method serves just for testing purposes; anyway, I modified it - does it look better now?

Copy link
Contributor

@dmitry-markin dmitry-markin Sep 5, 2022

Choose a reason for hiding this comment

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

Thanks — to be honest, I'm not familiar with this code, nor that it's for testing only. Just noticed that previously non-panicking code stopped to be such.

Copy link
Contributor Author

@pmikolajczyk41 pmikolajczyk41 Sep 5, 2022

Choose a reason for hiding this comment

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

yeah sure, that's a good point

Copy link
Contributor

@niklasad1 niklasad1 Sep 5, 2022

Choose a reason for hiding this comment

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

either way is fine for me,

The rpc connections is only used for CLI integrations tests and if the connection fails means that RPC server hasn't started or the URL is wrong.

Now we try to connect forever as it was before so fine which would be caught by the CI :)


loop {
if let Ok(block) = get_finalized_head::<Block, _>(url).await {
if let Ok(block) = rpc_service.get_finalized_head::<Block>().await {
built_blocks.insert(block);
if built_blocks.len() > n {
break
Expand Down