-
Couldn't load subscription status.
- Fork 0
feat: implement max/min req/res sizes #10
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
Conversation
we need to prepare for implementation of FCUs deduplication (this avalanche problem still persists). that will require assembly of incoming requests before the decision whether to proxy them or not is made => and for that we will need buffers + reasonable limits.
|
|
||
| let bck_req = this.backend.new_backend_request(&info); | ||
| let bck_req_body = ProxyHttpRequestBody::new(this.clone(), info, cli_req_body, timestamp); | ||
| let bck_req_body = ProxyHttpRequestBody::new( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wdyt about using full name?
i could read this as:
block_req_body
backend_req_body
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed:
bck=>bkndcli=>clnt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do these shortenings add? It's more straightforward to use full names for these sorts of variables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they make it easier for me to see more code on a single screen of my laptop.
rust is extremely verbose language, especially with autoformatting rules applied.
| if let Some(info) = mem::take(this.info) { | ||
| warn!( | ||
| proxy = P::name(), | ||
| request_id = %info.id(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to include connection id here too or was not including it intentional? I don't understand why you need to repeat this code instead of defining a function or something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I did mean to include it. just overlooked it. thanks for pointing out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re. function: maybe some day that will happen. I don't want to optimise things prematurely. once the code stabilised we can think about it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
responded
| if let Some(info) = mem::take(this.info) { | ||
| warn!( | ||
| proxy = P::name(), | ||
| request_id = %info.id(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I did mean to include it. just overlooked it. thanks for pointing out
| if let Some(info) = mem::take(this.info) { | ||
| warn!( | ||
| proxy = P::name(), | ||
| request_id = %info.id(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re. function: maybe some day that will happen. I don't want to optimise things prematurely. once the code stabilised we can think about it
|
|
||
| let bck_req = this.backend.new_backend_request(&info); | ||
| let bck_req_body = ProxyHttpRequestBody::new(this.clone(), info, cli_req_body, timestamp); | ||
| let bck_req_body = ProxyHttpRequestBody::new( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed:
bck=>bkndcli=>clnt
we need to prepare for implementation of FCUs deduplication (this avalanche problem still persists).
that will require assembly of incoming requests before the decision whether to proxy them or not is made => and for that we will need buffers + reasonable limits.