vcrpy icon indicating copy to clipboard operation
vcrpy copied to clipboard

_before_record_request is called twice in recording mode

Open jiasli opened this issue 5 years ago • 1 comments
trafficstars

_before_record_request is called twice in recording mode

In recording mode, it seems _before_record_request is called twice.

In can_play_response_for, _before_record_request is already called on the request instance at L253.

https://github.com/kevin1024/vcrpy/blob/535efe1eb92e894ccadc5515e0642b058c8c31f0/vcr/cassette.py#L252-L254

Then the in expression at L254 calls

https://github.com/kevin1024/vcrpy/blob/535efe1eb92e894ccadc5515e0642b058c8c31f0/vcr/cassette.py#L349-L351

which then calls https://github.com/kevin1024/vcrpy/blob/535efe1eb92e894ccadc5515e0642b058c8c31f0/vcr/cassette.py#L242-L247

Here _before_record_request is called again.

If filter_functions are not idempotent, this may cause failure.

Evaluation order

Also performance-wise, wouldn't it be nice to evaluate "cheap" conditions (like self.record_mode != RecordMode.ALL and self.rewound) first, then "expensive" conditions (request in self)?

As per Boolean operations:

The expression x and y first evaluates x; if x is false, its value is returned; otherwise, y is evaluated and the resulting value is returned.

https://github.com/kevin1024/vcrpy/blob/535efe1eb92e894ccadc5515e0642b058c8c31f0/vcr/cassette.py#L254

In such case, if any of self.record_mode != RecordMode.ALL or self.rewound doesn't stand, can_play_response_for can return False immediately rather than evaluate the expensive in condition.

Sorry if I misunderstood the code flow.

jiasli avatar Jul 10 '20 07:07 jiasli

Ops. Looks like it has been reported before - https://github.com/kevin1024/vcrpy/issues/524.

jiasli avatar Jul 10 '20 07:07 jiasli