error_injection: replace boost::lexical_cast with std::from_chars #22164
Conversation
🔴 CI State: FAILURE✅ - Build Failed Tests (2/35576):
Build Details:
|
|
🔴 CI State: FAILURE✅ - Build Build Details:
|
|
@yaronkaikov what can I do about the offline installer failures? |
It's scylladb/scylla-cluster-tests#9645 Generally, I think it's wrong to run them in CI. |
Well, just not long ago, we had a regression that took us a few months to fix. |
Run it nightly, or on next. Nor per PR. |
This is what we were doing, before (nightly) and it didn't help us much About running in |
| @@ -13,6 +13,7 @@ | |||
| #include <boost/range/adaptor/map.hpp> | |||
| #include <boost/range/algorithm/copy.hpp> | |||
| #include <boost/range/join.hpp> | |||
| #include <boost/lexical_cast.hpp> | |||
There was a problem hiding this comment.
Why not convert the error injection point in override_snapshot_thresholds too?
There was a problem hiding this comment.
Because I want to keep the patch localized and not grow it.
Ok with that. |
|
Docker test failed too. What can I do to make CI pass? |
It failed due to scylladb/scylla-cluster-tests#9645 @fruch any idea what is going on ? |
|
Since podman now supports nested containers, perhaps we can test the container image in test.py. |
those tests are not just for docker image... |
|
We can test other packages too in containers. |
🟢 CI State: SUCCESS✅ - Build Build Details:
|
| @@ -163,7 +161,14 @@ class error_injection { | |||
| if constexpr (std::is_same_v<T, std::string_view>) { | |||
| return s; | |||
| } else { | |||
| return boost::lexical_cast<T>(s.data(), s.size()); | |||
There was a problem hiding this comment.
you replaced 1 line of code with 8 lines of code. I think it's a change for the worse.
Would be better if those 8 lines were extracted to a utility template function
There was a problem hiding this comment.
std::from_chars interface is quite chatty, since it tries to be more flexible than lexical_cast.
I'll see if I can improve it.
avikivity
force-pushed
the
error_injection-lexical_cast
branch
from
5e54b22 to
b7c8d91
Compare
|
v2: introduce from_chars_exactly() to reduce noise |
🔴 CI State: FAILURE✅ - Build Failed Tests (1/36895):Build Details:
|
This is a replacement for boost::lexical_cast (but without its long dependency chain). It wraps std::from_chars(), providing a less flexible but also more concise interface.
Replace boost with a standard facility; this reduces dependencies as lexical_cast depends on boost ranges. As a side effect the exception error message is improved.
avikivity
force-pushed
the
error_injection-lexical_cast
branch
from
b7c8d91 to
9ff6473
Compare
|
v3: improve license for new file |
🟢 CI State: SUCCESS✅ - Build Build Details:
|
|
@nyh ping for merge |
Replace boost with a standard facility; this reduces dependencies as lexical_cast depends on boost ranges.
Since std::from_chars() is chatty, we introduce utils::from_chars_exactly() to trade some flexibility for conciseness.
Small build time improvement, no backport needed.