Sends auction_id as query parameter to http solvers. #133
Conversation
Codecov Report
@@ Coverage Diff @@
## main #133 +/- ##
==========================================
- Coverage 65.11% 65.10% -0.01%
==========================================
Files 182 182
Lines 38049 38047 -2
==========================================
- Hits 24775 24772 -3
- Misses 13274 13275 +1 |
| @@ -108,6 +106,10 @@ impl HttpSolverApi for DefaultHttpSolverApi { | |||
| use_internal_buffers.to_string().as_str(), | |||
| ); | |||
| } | |||
| if let Some(auction_id) = maybe_auction_id { | |||
| url.query_pairs_mut() | |||
| .append_pair("auction_id", auction_id.to_string().as_str()); | |||
There was a problem hiding this comment.
Do the MIP and CowDexAg solver support this parameter?
There was a problem hiding this comment.
Not MIP (which is being deprecated btw), not sure about cowdex.
There was a problem hiding this comment.
Should this also use the same fallback value for auction_id as the instance name so that the query parameter always gets added?
There was a problem hiding this comment.
Cow does not yet. But after I get an approval in this PR, it should be good:
gnosis/cow-dex-solver#18
crates/shared/src/http_solver.rs
Outdated
| let maybe_auction_id = model | ||
| .metadata | ||
| .as_ref() | ||
| .unwrap_or(&MetadataModel::default()) | ||
| .auction_id; | ||
| let instance_name = self.generate_instance_name(maybe_auction_id.unwrap_or(0u64)); |
There was a problem hiding this comment.
nit: The value of maybe_auction_id can depend on the implementation of MetadataModel::default() which is probably not intended. As an alternative you could try this:
let auction_id = match &model.metadata {
Some(metadata) => metadata.auction_id.unwrap_or(0),
None => 0,
};I believe this makes a little more obvious what's going on.
Alternatively if you actually want this value to be an Option<u64> this would also work:
let maybe_auction_id = model.metadata.as_ref().and_then(|data| data.auction_id);There was a problem hiding this comment.
I'll go with the last one, thank you!
| @@ -108,6 +106,10 @@ impl HttpSolverApi for DefaultHttpSolverApi { | |||
| use_internal_buffers.to_string().as_str(), | |||
| ); | |||
| } | |||
| if let Some(auction_id) = maybe_auction_id { | |||
| url.query_pairs_mut() | |||
| .append_pair("auction_id", auction_id.to_string().as_str()); | |||
There was a problem hiding this comment.
Should this also use the same fallback value for auction_id as the instance name so that the query parameter always gets added?
At least quasimodo is counting for the presence/absence of auction_id to decide which log info should be appended - if not present then it records an internal request counter. So I'd say to keep it like this if all agree. |
No description provided.