feat: implement the /configuration endpoint #39
Conversation
Cargo.toml
Outdated
| @@ -29,7 +29,7 @@ mozsvc-common = "0.1.0" | |||
| num_cpus = "1.8.0" | |||
| rand = "0.5.2" | |||
| ring = "0.13.2" | |||
| serde = "1.0.67" | |||
| serde = { version = "1.0.67", features = ["rc"] } | |||
There was a problem hiding this comment.
Feature added so that Arc<ServerLimits> can be serialized to a JSON response payload. The alternative is to derive Clone and Copy on ServerLimits and don't keep it in an Arc. I wasn't sure which would be preferable, happy to make the switch if this way is wrong.
There was a problem hiding this comment.
You should be able to get away w/out it. The inner ServerLimits can be deref'd out of the arc via *server.limits -- and we need to borrow it for the json call, so: json(&*server.limits)
Feel free to change this later if it makes it easier on these 2 outstanding PRs
| test_endpoint( | ||
| http::Method::GET, | ||
| "/42/info/configuration", | ||
| &serde_json::to_string(&ServerLimits::default()).unwrap(), |
There was a problem hiding this comment.
I did wonder whether this test is too tautological to be useful, but I think it's okay. Asserting that the response payload equals the JSON serialization of the default ServerLimits is fine, yeah?
| static DEFAULT_MAX_RECORD_PAYLOAD_BYTES: u32 = 2 * MEGABYTE; | ||
| static DEFAULT_MAX_REQUEST_BYTES: u32 = DEFAULT_MAX_POST_BYTES + 4 * KILOBYTE; | ||
| static DEFAULT_MAX_TOTAL_BYTES: u32 = 100 * DEFAULT_MAX_POST_BYTES; | ||
| static DEFAULT_MAX_TOTAL_RECORDS: u32 = 100 * DEFAULT_MAX_POST_RECORDS; |
There was a problem hiding this comment.
There was a problem hiding this comment.
We should probably retain the doc strings as well.
| #[derive(Debug)] | ||
| pub struct Secrets { | ||
| pub master_secret: Vec<u8>, | ||
| pub signing_secret: [u8; 32], | ||
| } | ||
|
|
||
| impl Secrets { | ||
| pub fn new(master_secret: &str) -> Secrets { | ||
| pub fn new(master_secret: &str) -> Self { |
There was a problem hiding this comment.
I only learned this for the first time when I reviewed one of those pull requests yesterday. Hence going back and applying it to the other code I wrote in here, even though it's not strictly related.
|
It just occurred to me that I haven't done anything in this PR to actually enforce these limits, I'm just blithely using the same values as the Python codebase. Is it something that we'd configure in nginx or whatever, or are there hooks in actix-web we can use? (I'll go and RTFM right now, sorry) Anyway, hopefully it's okay for a follow-up (I'll open an issue if so)... EDIT: #40 |
(I mean the EDIT: Actually, thinking it through sorry, it's only |
|
Sorry both, I had to rebase because merge conflicts, so your reviews were dismissed. On the plus side it was an opportunity to try Phil's suggestion about dereffing |
Fixes #25.
Adds a
ServerLimitstype tosrc/settings.rs, containing the static values returned by the/info/configurationendpoint. The default values are the same as the ones in the Python code.@bbangert @pjenvey r?