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.

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

Cherry-pick Seastar patch #15664

Closed
wants to merge 1 commit into from
Closed

Cherry-pick Seastar patch #15664

wants to merge 1 commit into from

Conversation

nyh
Copy link
Contributor

@nyh nyh commented Oct 8, 2023

Backported Seastar commit 4f4e84bb2cec5f11b4742396da7fc40dbb3f162f:

  • seastar 04a39f448...06bb98796 (1):

    sstring: refactor to_sstring() using fmt::format_to()

Refs #15127

Backported Seastar commit 4f4e84bb2cec5f11b4742396da7fc40dbb3f162f:

* seastar 04a39f448...06bb98796 (1):
  > sstring: refactor to_sstring() using fmt::format_to()

Refs scylladb#15127
@scylladb-promoter
Copy link
Contributor

@tchaikov tchaikov added this to the 5.1 milestone Oct 9, 2023
@tchaikov
Copy link
Contributor

tchaikov commented Oct 9, 2023

 pytest failurea pytest failed:  json_test.py::TestsToJsonSelect::test_basic_data_types

the test failure is expected. but we need to address it on the dtest's end. the change which originally brought the alternative representation of float formatting was https://github.com/scylladb/scylla-dtest/pull/2982. it was more than a refactory. actually it included two kinds of changes:

  1. changes which migrated json_test.py from doctest
  2. changes for supporting both the new representation.

so we need to backport the whole change to the branch-5.1 branch, so that the test in question can pass before and after this change. yeah, we will drop the old one after this PR lands, like https://github.com/scylladb/scylla-dtest/commit/9c2a868be9e71757dbf596489e6f299f0ecf1899

created at https://github.com/scylladb/scylla-dtest/pull/3620.

EDIT, the PR above got merged. retriggering the build and test 🤞

@scylladb-promoter
Copy link
Contributor

nyh added a commit that referenced this pull request Oct 9, 2023
Backported Seastar commit 4f4e84bb2cec5f11b4742396da7fc40dbb3f162f:

* seastar 04a39f448...06bb98796 (1):
  > sstring: refactor to_sstring() using fmt::format_to()

Refs #15127

Closes #15664
@nyh
Copy link
Contributor Author

nyh commented Oct 10, 2023

@tchaikov
Copy link
Contributor

Reached branch-5.1. @fruch - dtest now fails - see https://jenkins.scylladb.com/job/scylla-5.1/job/next/325/, can you please take a look?

dtest / json_test.TestsFromJsonInsert.test_basic_data_types dtest / json_test.TestsToJsonSelect.test_basic_data_types

seems we were using next-5.1 for testing, while the backport went straight into branch-5.1: next-5.1 does not in include the backport.

@nyh
Copy link
Contributor Author

nyh commented Oct 10, 2023

Reached branch-5.1. @fruch - dtest now fails - see https://jenkins.scylladb.com/job/scylla-5.1/job/next/325/, can you please take a look?
dtest / json_test.TestsFromJsonInsert.test_basic_data_types dtest / json_test.TestsToJsonSelect.test_basic_data_types

seems we were using next-5.1 for testing, while the backport went straight into branch-5.1: next-5.1 does not in include the backport.

Which repository are you referring when you are giving these branch names? This backport in this did not PR go "straight to branch-5.1. It went into scylla.git next-5.1, it wasn't yet promoted to scylla.git's branch-5.1. This is tested against some branch of scylla-dtest.git, I'm not sure which and I want @fruch to take a look at it. It's possible that @fruch's fix didn't make it to scylla-dtest.git's branch-5.1 and this is what Jenkins is using. He may need to do his change in two steps - one that will be able to reach scylla-dtest.git's branch-5.1 (because it passes also on the older version of Scylla, by disabling the test). And only when all the dust settles, a second PR to restore the test.

@tchaikov
Copy link
Contributor

i am referring scylla-dtest repo.

@fruch
Copy link
Contributor

fruch commented Oct 10, 2023

i am referring scylla-dtest repo.

@nyh, @tchaikov is correct I've failed to notice that https://github.com/scylladb/scylla-dtest/pull/3620 is pointed straight at branch-5.1, and not on next-5.1

the code that supposed to warn/block me about those, isn't in 5.1 😩

I'll fix is shortly

@nyh
Copy link
Contributor Author

nyh commented Oct 10, 2023

This PR reached branch-5.1, and will not be closed automatically, so I'm doing it manually.

@nyh nyh closed this Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants