Add serde_urlencoded to db-example Cargo.toml

[jaztec]

No real changes, just added serde_urlencoded to the db-example sub-project

I license past and future contributions under the dual MIT/Apache-2.0 license, allowing licensees to chose either at their option.

[HeroicKatora]

I wonder what's the point here as nothing depends on it directly? To influence version resolution of actix-http and awc (two transitive dependencies)? Since no Cargo.lock file is committed this will be completely redundant for everyone. And in case these two crates migrate to a newer version the example depends on an unecessary library.

[jaztec]

That is true. However when I tried to cargo build this library from the root directory I got slapped on the wrist by rocket (I use 1.55.0 stable and rocket requires nightly as depicted in the docs) and this error on serde_urlencoded which is a dependency but not listed in Cargo.toml.

Listing it fixed it for me. But you are right about there has not been an apparent lock file generated. Maybe I need to get into the rust ecosystem a bit more first 😅

[jaztec]

Before I retract the pull request, I guess the problem exists in generic module judging by the error message:

warning: `oxide-auth-actix` (lib) generated 1 warning
   Compiling db-example v0.1.0 (/home/jasper/Projects/jaztec/oxide-auth/oxide-auth-db/examples/db-example)
error[E0433]: failed to resolve: use of undeclared crate or module `serde_urlencoded`
  --> oxide-auth-db/examples/db-example/src/../../../../examples/support/generic.rs:80:9
   |
80 |         serde_urlencoded::to_string(extra).unwrap(),
   |         ^^^^^^^^^^^^^^^^ use of undeclared crate or module `serde_urlencoded`

The listing inside the Cargo.toml fixes this.

But again, like I said. I'm just starting out with rust and am probably looking at the problem the wrong way.

[jaztec]

Reopened just for the sake of thinking it being directly required anyway 😄

[HeroicKatora]

Huh, I was running on the assumption that CI would have complained about that one but you're right. Okay, I'm going to push an update to the CI scripts as well then :)

[jaztec]

Well now you broke everything 😆.

[HeroicKatora]

I feared so. I'm about to try and unbreak it.

[jaztec]

I'm on it as well, I think cirrus just needs an additional container for Redis https://github.com/jaztec/oxide-auth/blob/master/oxide-auth-db/src/primitives/db_registrar.rs#L190

[jaztec]

The tests succeed for me locally but I'm running Redis locally as well

[HeroicKatora]

I think it would be easier to just skip the test on CI than to add an extra dependency there. At least that's easy to configure. Maybe you can add the CI configuration with in a separate PR? I really like to keep everything else lightweight as possible.

[jaztec]

I guess the host mapping is wrong, which makes sense. But changing that would mean local tests failing when you don't have a mapping in your hosts file.

Just made a push with (hopefully) a cirrus skip test. Not sure I've got the syntax right though

[jaztec]

Apparently not

[HeroicKatora]

Let's try a literal environment flag in tests then ^^

[jaztec]

It seems your rust foo is doing the trick