feat: support poem v2.0.0

[paulotten]

Description of change

Closes #1519.

Adds support for poem v2.0.0.

Approach is based on how support for Axum 0.7 was added to shuttle-axum in 0.34.1

How has this been tested? (if applicable)

Using /examples/poem/hello-world/, I did cargo check with the following changes to dependencies

Poem 1 (regression test)

poem = "1.3.55"
shuttle-poem = { path = "../../../services/shuttle-poem/" }

Poem 2

poem = "2.0.0"
shuttle-poem = { path = "../../../services/shuttle-poem/", default-features = false, features = ["poem-2"] }

[oddgrd]

Cool, thanks a lot, Paul! I am wondering, though, if we need to take this step for Poem. From the changelog it seems like there are very few changes except for the upgrade of Hyper, so as far as I can tell the user won't need to make updates to their application, we just need to update the part that starts the server on our end in shuttle-poem. Oh, and they'll need to update their version of poem if they update their version of shuttle-poem. Or am I missing something here?

[paulotten]

Oh, and they'll need to update their version of poem if they update their version of shuttle-poem. Or am I missing something here?

The version of poem (poem = "1.3.55" vs poem = "2.0.0") used in the example/user application needs to match the version of poem in shuttle-poem.

Otherwise you get the

#[shuttle_runtime::main]
^^^^^^^^^^^^^^^^^^^^^^^^ the trait `poem::endpoint::endpoint::Endpoint` is not implemented for `impl Endpoint`

error reported in #1519.

This appears to be the entirety of the issue.

(It's also as I mentioned a breaking change since the user must update their version of poem at the same time or their app will break.)

[paulotten]

Re: failed CI check

cargo clippy --tests \
             --all-targets \
             --all-features \
             --manifest-path services/shuttle-poem/Cargo.toml \
             --no-deps -- \
             --D warnings

error[E0252]: the name `poem` is defined multiple times
 --> src/lib.rs:8:5
  |
6 | use poem_1 as poem;
  |     -------------- previous import of the module `poem` here
7 | #[cfg(feature = "poem-2")]
8 | use poem_2 as poem;
  |     ^^^^^^^^^^^^^^ `poem` reimported here
[...]

The issue is the --all-features flag. Features poem-1 and poem-2 are mutually exclusive.

(It looks like the current version of shuttle-axum would also fail such a test. https://docs.rs/shuttle-axum/latest/src/shuttle_axum/lib.rs.html#5-8)

[oddgrd]

Yes, but if they are already bumping the version of shuttle-poem, if bumping poem as well does not require any changes to their application, do we need feature flags to avoid forcing people to upgrade? Axum 0.7 also came with other breaking changes, so we didn't want to force people to upgrade if they weren't prepared to upgrade their application. I'm not sure we need to take the same approach here. 🤔

[paulotten]

Yes, but if they are already bumping the version of shuttle-poem, if bumping poem as well does not require any changes to their application, do we need feature flags to avoid forcing people to upgrade? Axum 0.7 also came with other breaking changes, so we didn't want to force people to upgrade if they weren't prepared to upgrade their application. I'm not sure we need to take the same approach here. 🤔

If you increase the version of shuttle-poem to 0.37.0 then that should be ok.

If you only increase the version of shuttle-poem to 0.36.1 then you are going to have problems. Someone will checkout an example that uses shuttle-poem = 0.36.0 and poem = "1.3.55". Cargo will select shuttle-poem 0.36.1 (with the change) as compatible with 0.36.0. The user will not be able to compile the example due to the above error.

https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-cratesio

[paulotten]

There is also the question of if Shuttle wants to provide backwards support for poem 1 in shuttle-poem 0.37 and later. (Which is not a question I can answer.)

[jonaro00]

I agree that the changes in Poem 2 are too minor to justify having feature flag support for both versions at the same time.
A release for 0.36.1 is unlikely to happen, so I think we can simply bump poem to v2 and aim for 0.37.0 with no backwards compatibility.

[jonaro00]

Thanks! A PR that does the same thing for the poem examples (onto the develop branch) would be greatly appreciated! (don't worry about the shuttle crate version numbers or failing CI due to them)

[Mouwrice]

Hi hi, original feature requester here 👋
Thanks for the speedy resolution @paulotten !

Just wanted to say that I have tested these changes and everything works as it should. As mentioned in this thread, poem 2.0.0 actually has little changes and in my case no changes needed to be made to my (small) codebase.

[oddgrd]

Great, thanks Paul! And thanks @Mouwrice for confirming the upgrade is easy on the user side!