Page MenuHomePhabricator
Differential D17242

[CHRONIK] Updated pyo3 version
ClosedPublic
Actions

Authored by hazzarust on Thu, Nov 28, 23:56.

Details

Reviewers
tobias_ruck
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABC3f69da396b5a: [CHRONIK] Updated pyo3 version
Summary

Updated the pyo3 module in chronik-plugin-impl/tests/Cargo.toml.

The old syntax PyModule::from_code_bound() takes in a &str. With the new change to PyModule::from_code(), we need to take in a &CStr.

Therefore, we use CString::new() that returns a result<CString, NulError>. CString is a struct that wraps around a Vec<u8>. We then convert this using as_c_str() which returns a &CStr, which now satisfies PyModule::from_code_bound() parameter rules.

The other changes to call, new, import and empty are only renamed to keep the syntax up to date.

Test Plan

ninja check-functional && ninja check-crates

Diff Detail

Event Timeline

hazzarust created this revision.Thu, Nov 28, 23:56
Owners added a reviewer: Restricted Owners Package.Thu, Nov 28, 23:56
Herald added a reviewer: Restricted Project. · View Herald TranscriptThu, Nov 28, 23:56
hazzarust requested review of this revision.Thu, Nov 28, 23:56
tobias_ruck requested changes to this revision.Thu, Nov 28, 23:58
tobias_ruck added a subscriber: tobias_ruck.
tobias_ruck added inline comments.
chronik/chronik-plugin-impl/tests/test_tx_to_py.rs
170 ↗(On Diff #51197)

I don't think this will work

This revision now requires changes to proceed.Thu, Nov 28, 23:58
Harbormaster completed remote builds in B31391: Diff 51197.Fri, Nov 29, 00:05
teamcity added a comment.Fri, Nov 29, 00:05
Build Bitcoin ABC Diffs / Diff Testing (build-chronik) failed.

Tail of the build log:

test error::tests::test_report_error ... ok
test protobuf::tests::test_protobuf ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (abc-ci-builds/build-chronik/cargo/build/debug/deps/chronik_indexer-da20dc22ba07e7eb)

running 8 tests
test merkle::tests::test_calc_branch_len_0_panics - should panic ... ok
test merkle::tests::test_calc_branch_len ... ok
test merkle::tests::test_branches ... ok
test merkle::tests::test_cache_levels ... ok
test merkle::tests::test_roots ... ok
test indexer::tests::test_plugin_versions ... ok
test indexer::tests::test_indexer ... ok
test indexer::tests::test_schema_version ... ok

test result: ok. 8 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 2.36s

     Running unittests src/lib.rs (abc-ci-builds/build-chronik/cargo/build/debug/deps/chronik_plugin-240df94ee51cf04d)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (abc-ci-builds/build-chronik/cargo/build/debug/deps/chronik_plugin_common-3e01def9313d8b79)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

     Running unittests src/lib.rs (abc-ci-builds/build-chronik/cargo/build/debug/deps/chronik_plugin_impl-3de61a5e113b105d)

running 1 test
test context::tests::test_run_on_tx ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.05s

     Running tests/test_tx_to_py.rs (abc-ci-builds/build-chronik/cargo/build/debug/deps/test_tx_to_py-58f7933ce38f4f0c)

running 1 test
test test_tx_to_py ... FAILED

failures:

---- test_tx_to_py stdout ----
Error: NameError: name 'test_tx_to_py' is not defined

Location:
    chronik/chronik-plugin-impl/tests/test_tx_to_py.rs:169:27


failures:
    test_tx_to_py

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.04s

error: test failed, to rerun pass `-p chronik-plugin-impl --test test_tx_to_py`
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik failed with exit code 1
teamcity added a comment.Fri, Nov 29, 00:10
Build Bitcoin ABC Diffs / Diff Testing (build-chronik-plugins) failed.

Tail of the build log:

    Checking http-body-util v0.1.2
    Checking chronik-util v0.1.0 (/work/chronik/chronik-util)
   Compiling prost-types v0.11.9
    Checking tracing v0.1.40
    Checking sha1 v0.10.6
    Checking smallvec v1.13.2
    Checking ryu v1.0.18
    Checking httpdate v1.0.3
    Checking data-encoding v2.6.0
    Checking cobs v0.2.3
    Checking percent-encoding v2.3.1
    Checking utf-8 v0.7.6
    Checking form_urlencoded v1.2.1
    Checking postcard v1.0.10
    Checking tungstenite v0.24.0
    Checking serde_spanned v0.6.8
    Checking toml_datetime v0.6.8
   Compiling pyo3 v0.23.2
    Checking sync_wrapper v1.0.1
    Checking seahash v4.1.0
    Checking chronik-plugin-common v0.1.0 (/work/chronik/chronik-plugin-common)
    Checking winnow v0.6.20
    Checking mime v0.3.17
    Checking topo_sort v0.4.0
   Compiling prost-build v0.11.9
    Checking sync_wrapper v0.1.2
    Checking chronik-plugin v0.1.0 (/work/chronik/chronik-plugin)
    Checking axum-core v0.4.5
    Checking memoffset v0.9.1
    Checking serde_urlencoded v0.7.1
    Checking serde_json v1.0.133
    Checking futures-executor v0.3.31
    Checking serde_path_to_error v0.1.16
    Checking hyper v1.5.1
    Checking tower v0.5.1
    Checking tokio-tungstenite v0.24.0
    Checking base64 v0.22.1
    Checking matchit v0.7.3
    Checking unicode-segmentation v1.12.0
    Checking hyper-util v0.1.10
    Checking unindent v0.2.3
    Checking tower-http v0.5.2
    Checking thiserror v2.0.3
    Checking futures v0.3.31
    Checking toml_edit v0.22.22
    Checking convert_case v0.6.0
   Compiling chronik-proto v0.1.0 (/work/chronik/chronik-proto)
   Compiling chronik-bridge v0.1.0 (/work/chronik/chronik-bridge)
    Checking axum v0.7.9
   Compiling librocksdb-sys v0.11.0+8.1.1
   Compiling pyo3-macros v0.23.2
    Checking toml v0.8.19
    Checking chronik-plugin-impl v0.1.0 (/work/chronik/chronik-plugin-impl)
    Checking rocksdb v0.21.0
    Checking chronik-db v0.1.0 (/work/chronik/chronik-db)
    Checking chronik-indexer v0.1.0 (/work/chronik/chronik-indexer)
    Checking chronik-http v0.1.0 (/work/chronik/chronik-http)
    Finished dev [unoptimized + debuginfo] target(s) in 7m 08s
ninja: build stopped: cannot make progress due to previous errors.
Build build-chronik-plugins failed with exit code 1
hazzarust updated this revision to Diff 51198.Fri, Nov 29, 00:21

Added the include_str! macro and updated the module_name to its original name "test_tx_to_py"

hazzarust edited the summary of this revision. (Show Details)Fri, Nov 29, 00:32
hazzarust edited the test plan for this revision. (Show Details)
hazzarust edited the summary of this revision. (Show Details)
Harbormaster completed remote builds in B31392: Diff 51198.Fri, Nov 29, 00:45
teamcity added a comment.Fri, Nov 29, 00:45
Build Bitcoin ABC Diffs / Diff Testing (build-chronik) failed.

Failed tests logs:

====== Bitcoin ABC functional tests: chronik_resync.py ======

------- Stdout: -------
2024-11-29T00:40:43.770000Z TestFramework (INFO): Initializing test directory /work/abc-ci-builds/build-chronik/test/tmp/test_runner_₿₵_🏃_20241129_003623/chronik_resync_66
2024-11-29T00:40:53.811000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
  File "/work/test/functional/test_framework/test_framework.py", line 148, in main
    self._run_test_internal()
  File "/work/test/functional/test_framework/test_framework.py", line 138, in _run_test_internal
    self.run_test()
  File "/work/test/functional/chronik_resync.py", line 108, in run_test
    chronik.block(100).ok()
  File "/work/test/functional/test_framework/chronik/client.py", line 30, in ok
    raise AssertionError(
AssertionError: Expected OK response, but got status 404, error: msg: "404: Block not found: 100"

2024-11-29T00:40:53.862000Z TestFramework (INFO): Stopping nodes
2024-11-29T00:40:54.264000Z TestFramework (WARNING): Not cleaning up dir /work/abc-ci-builds/build-chronik/test/tmp/test_runner_₿₵_🏃_20241129_003623/chronik_resync_66
2024-11-29T00:40:54.264000Z TestFramework (ERROR): Test failed. Test logging available at /work/abc-ci-builds/build-chronik/test/tmp/test_runner_₿₵_🏃_20241129_003623/chronik_resync_66/test_framework.log
2024-11-29T00:40:54.264000Z TestFramework (ERROR): 
2024-11-29T00:40:54.264000Z TestFramework (ERROR): Hint: Call /work/test/functional/combine_logs.py '/work/abc-ci-builds/build-chronik/test/tmp/test_runner_₿₵_🏃_20241129_003623/chronik_resync_66' to consolidate all logs
2024-11-29T00:40:54.264000Z TestFramework (ERROR): 
2024-11-29T00:40:54.264000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-11-29T00:40:54.264000Z TestFramework (ERROR): https://github.com/Bitcoin-ABC/bitcoin-abc/issues
2024-11-29T00:40:54.264000Z TestFramework (ERROR):

Each failure log is accessible here:
Bitcoin ABC functional tests: chronik_resync.py

hazzarust updated this revision to Diff 51215.Fri, Nov 29, 10:35
hazzarust edited the summary of this revision. (Show Details)
hazzarust edited the test plan for this revision. (Show Details)

rerun ci

Harbormaster completed remote builds in B31395: Diff 51215.Fri, Nov 29, 10:58
tobias_ruck requested changes to this revision.Fri, Nov 29, 11:08
tobias_ruck added inline comments.
chronik/chronik-plugin-impl/src/util.rs
16

static_str is not a good name because the str isn't static

16

seems like just using CString::new(...)? works too?

chronik/chronik-plugin-impl/tests/test_tx_to_py.rs
33–34

combine these two

171

doesn't this work? same for the others

This revision now requires changes to proceed.Fri, Nov 29, 11:08
hazzarust updated this revision to Diff 51217.Fri, Nov 29, 12:44

Reverted back to just calling CString instead of using the new str_to_cstr function

hazzarust edited the summary of this revision. (Show Details)Fri, Nov 29, 13:01
Harbormaster completed remote builds in B31397: Diff 51217.Fri, Nov 29, 13:11
tobias_ruck requested changes to this revision.Fri, Nov 29, 15:39
tobias_ruck added inline comments.
chronik/chronik-plugin-impl/Cargo.toml
32 ↗(On Diff #51217)

seems out of scope here?

let's do that in another diff

chronik/chronik-plugin-impl/tests/test_tx_to_py.rs
5–6 ↗(On Diff #51217)

combine those two

This revision now requires changes to proceed.Fri, Nov 29, 15:39
hazzarust updated this revision to Diff 51230.Fri, Nov 29, 15:57

Reformatted the use statement to make it one line
use std::{collections::BTreeMap, ffi::CString};
Also changed thiserror back to original version (1.0), and will change in a seperate diff

Harbormaster completed remote builds in B31405: Diff 51230.Fri, Nov 29, 16:59
tobias_ruck accepted this revision.Fri, Nov 29, 17:06
This revision is now accepted and ready to land.Fri, Nov 29, 17:06
Closed by commit rABC3f69da396b5a: [CHRONIK] Updated pyo3 version (authored by hazzarust). · Explain WhySat, Nov 30, 19:05
This revision was automatically updated to reflect the committed changes.
hazzarust added a commit: rABC3f69da396b5a: [CHRONIK] Updated pyo3 version.

Revision Contents
Changeset List

PathSize
chronik/
chronik-plugin-impl/
4 lines
src/
6 lines
14 lines
2 lines
52 lines
4 lines
39 lines
11 lines
tests/
9 lines

Diff 51215

View Options

Cargo.lock

Loading...
View Options

chronik/chronik-plugin-impl/Cargo.toml

Loading...
View Options

chronik/chronik-plugin-impl/src/context.rs

Loading...
View Options

chronik/chronik-plugin-impl/src/etoken.rs

Loading...
View Options

chronik/chronik-plugin-impl/src/lib.rs

Loading...
View Options

chronik/chronik-plugin-impl/src/module.rs

Loading...
View Options

chronik/chronik-plugin-impl/src/plugin.rs

Loading...
View Options

chronik/chronik-plugin-impl/src/tx.rs

Loading...
View Options

chronik/chronik-plugin-impl/src/util.rs

Loading...
View Options

chronik/chronik-plugin-impl/tests/test_tx_to_py.rs

Loading...