mozilla-services / syncstorage-rs

Sync Storage server in Rust
Mozilla Public License 2.0
973 stars 49 forks source link

Upgrading to Actix 4.x #1514

Closed bendk closed 9 months ago

bendk commented 9 months ago

Description

Updating dependencies:

The unit tests are passing for me, but I'm not sure if this is ready to merge yet. One reason I'm making a PR is that I think it will run the integration tests, which I gave up on getting to work on my machine.

I'm not sure how to land these changes. I think some of these changes can be factored out and landed without upgrading any dependencies, I'll try to open a PR for that. Other than that, I'm not sure if this can be split apart more. I don't think it makes sense to upgrade any one of the dependencies without upgrading the others since that will result it duplicate crates getting compiled in (for example, the tests fail if you don't upgrade reqwest because the tokio runtimes get mixed up).

Issue(s)

Closes #1249 Closes #1258

bendk commented 9 months ago

I think some of these changes can be factored out and landed without upgrading any dependencies, I'll try to open a PR for that.

After testing it out some more, I don't think there's much than can be done before the upgrades.

bendk commented 9 months ago

Updated the code to use insert_header, ran cargo update and removed the audit ignores. I'm starting to feel good about this one, although I don't have a good way to judge the risk. What should be the next step?

jrconlin commented 9 months ago

This looks like a fairly light lift. For heavier things, we'd want to run a load test to see if there were any large impacts to performance, but I don't think that's a concern here since the upgrades aren't impacting much of the code flow or design. (Mostly it's function name changes and a bit of ownership changes.)

If you're interested, the somewhat crufty load test script we have is here, I'll reach out to Kat to see if there's a more recent script.

jrconlin commented 9 months ago

sigh I really wish that github would flag unsigned commits earlier rather than as the very last check.