vcrpy icon indicating copy to clipboard operation
vcrpy copied to clipboard

'rerecord' mode that essentially ignores the recorded cassette, strict validation of record modes

Open colonelpanic8 opened this issue 10 years ago • 30 comments

Is there a reason that this mode doesn't exist? Perhaps because you can simply delete the existing cassette and rerecord?

colonelpanic8 avatar Sep 13 '15 01:09 colonelpanic8

IIRC I pretty much just ripped off the modes from Ruby VCR. That said, no reason we can't add more if there's demand.

kevin1024 avatar Sep 13 '15 01:09 kevin1024

Doesn't all already do this? https://vcrpy.readthedocs.org/en/latest/usage.html#all

agriffis avatar Nov 24 '15 22:11 agriffis

@agriffis not exactly. I think all will append to the existing cassette instead of completely rewriting it. I might be wrong about that.

colonelpanic8 avatar Nov 25 '15 01:11 colonelpanic8

Nah, all is "Never replay previously recorded interactions. This can be temporarily used to force VCR to re-record a cassette"

The mode you're thinking of is new_episodes

Pretty sure that all is the same as rerecord

agriffis avatar Nov 25 '15 02:11 agriffis

Nah, all is "Never replay previously recorded interactions. This can be temporarily used to force VCR to re-record a cassette"

The mode you're thinking of is new_episodes

Pretty sure that all is the same as rerecord

From what I can tell from reading the code, the only place where all has a different effect than new_episodes is

https://github.com/kevin1024/vcrpy/blob/8a5bf23d3448892ea10a29b5392927d0a8784271/vcr/cassette.py#L273

Given that new episodes DOES preserve existing records, I'm not sure how it would be possible for all mode NOT to preserve existing records.

In fact, there does not seem to be anything special about the string 'new_episodes' at all -- you could use any other string that isn't once, none or all and acheive the same effect.

colonelpanic8 avatar Nov 25 '15 07:11 colonelpanic8

Ugh, you're right, all doesn't behave like I thought. It appends to the cassette, even though the doc implies that it should be overwriting.

And that sucks about using any string, because it means a misspelling will be interpreted as new_episodes. It should be a strict validation and raise ValueError if not recognized.

agriffis avatar Nov 25 '15 14:11 agriffis

Ugh, you're right, all doesn't behave like I thought. It appends to the cassette, even though the doc implies that it should be overwriting.

As such, do you think we should consider this a bug? Is the current functionality of 'all' something someone could conceivably want? @kevin1024 I'd like to hear your thoughts too. I think we should change the docstring for 'all' at the very least.

And that sucks about using any string, because it means a misspelling will be interpreted as new_episodes. It should be a strict validation and raise ValueError if not recognized.

Yeah this is definitely something that I'd like to change as well.

colonelpanic8 avatar Nov 25 '15 14:11 colonelpanic8

And that sucks about using any string, because it means a misspelling will be interpreted as new_episodes. It should be a strict validation and raise ValueError if not recognized.

:+1: Yes, this should be strictly validated

Ugh, you're right, all doesn't behave like I thought. It appends to the cassette, even though the doc implies that it should be overwriting.

I ripped off all the record modes from ruby vcr. Here's what it says about the all record mode:

The :all record mode records all requests and does not replay any previously recorded responds.

This can be temporarily used to force VCR to re-record a cassette (i.e. to ensure the responses are not out of date) or can be used when you simply want to log all HTTP requests.

So it sounds like it should be overwriting old requests that match. But what does it do to the requests already in the cassette that don't match? Are they effectively deleted?

kevin1024 avatar Nov 25 '15 15:11 kevin1024

Hi, just started using this today (and think it's pretty great), but it doesn't quite work for me and I think a "rerecord" mode (or altered "all" mode) would solve my problem. The way my app works, a search request is sent by the FE, a search ID is returned by the BE, and then the FE polls that search ID until the search results are ready. If I record this interaction and the search results aren't ready right away, my cassette essentially contains no search results when I switch to "none" mode. This is because the response with no results is repeated until my FE times out waiting for results. But if the poll request was overwritten each time during record, I would have what I wanted during playback. Any suggestions? Is this a good use case for rerecord?

hellmanj avatar Jan 11 '16 22:01 hellmanj

Hi, just started using this today (and think it's pretty great), but it doesn't quite work for me and I think a "rerecord" mode (or altered "all" mode) would solve my problem. The way my app works, a search request is sent by the FE, a search ID is returned by the BE, and then the FE polls that search ID until the search results are ready. If I record this interaction and the search results aren't ready right away, my cassette essentially contains no search results when I switch to "none" mode. This is because the response with no results is repeated until my FE times out waiting for results. But if the poll request was overwritten each time during record, I would have what I wanted during playback. Any suggestions? Is this a good use case for rerecord?

vcrpy is typically used within an automated testing context. Are you saying that you are using vcrpy with an active server and an actual frontend?

It almost sounds like you are trying to use vcrpy as a caching mechanism. I might be way off base here, but I'm not really sure what you are asking.

colonelpanic8 avatar Jan 11 '16 23:01 colonelpanic8

Yes :) we want to use it for both mocking responses during tests and caching data, as you say, so we can use it during development without needing to rely on a backend which is unstable right now (I think they don't even know why yet...)

Anyway, I think it's kind of besides the point. The app makes the same request over and over until a favorable result is returned. I'd like vcrpy to overwrite the response until that happens. For now, I can edit the cassette to pick the response I want, so no big deal. Just trying to add a use case here.

hellmanj avatar Jan 11 '16 23:01 hellmanj

Yes :) we want to use it for both mocking responses during tests and caching data, as you say, so we can use it during development without needing to rely on a backend which is unstable right now (I think they don't even know why yet...)

Thats definitely not a use case one that was imagined when vcrpy was conceived, but I suppose that it is possible to use it to do this. With that said, it is unlikely that I or any of the other maintainers will make an active effort to support this functionality.

Anyway, I think it's kind of besides the point. The app makes the same request over and over until a favorable result is returned. I'd like vcrpy to overwrite the response until that happens.

The best way to do this would be to provide a custom before_record_response that returns None when the response should not be recorded you'll want to use the new_episodes record mode.

Note that I haven't packaged a version of vcrpy that allows response filtering yet. I just comitted the change that allows this. But it will make it in to the next version either (1.7.5 or 1.8).

Oh and all of this is pretty orthogonal to the topic of #208. Please continue discussion by filing a new issue if necessary.

colonelpanic8 avatar Jan 12 '16 06:01 colonelpanic8

It already mostly works great as a development cache, so no added support needed, just trying to figure out this one issue.

I tried your suggestion, but then I get a cassette with two interactions where each interaction's response is null. The favorable response isn't saved at all. I also get this trace:

...
  File "/path/to/env/lib/python3.4/site-packages/requests/packages/urllib3/connectionpool.py", line 378, in _make_request
    httplib_response = conn.getresponse()
  File "/path/to/env/lib/python3.4/site-packages/vcr/stubs/__init__.py", line 226, in getresponse
    return VCRHTTPResponse(response)
  File "/path/to/env/lib/python3.4/site-packages/vcr/stubs/__init__.py", line 72, in __init__
    self.reason = recorded_response['status']['message']
TypeError: 'NoneType' object is not subscriptable

Will the new response filtering solve this? If so, hope it comes soon!

hellmanj avatar Jan 12 '16 15:01 hellmanj

Decided to answer my own question, I just tried installing from master and it worked!

hellmanj avatar Jan 12 '16 15:01 hellmanj

Yeah -- as I mentioned above:

Note that I haven't packaged a version of vcrpy that allows response filtering yet. I just comitted the change that allows this. But it will make it in to the next version either (1.7.5 or 1.8).

colonelpanic8 avatar Jan 12 '16 19:01 colonelpanic8

sorry to jump in, I do need this feature which the all mode still append to the existing cassette.

silencev avatar Jul 12 '17 09:07 silencev

I was caught by this too, my cassettes kept growing when I wanted to just refresh request/response data.

A "rerecord" mode (or any other name) would be fine, in the meantime I've just added my own persister which erases the cassette in "load_cassette()", only when record_mode="all".

I can submit a PR if I know which way is considered the best to solve this use case (changing the meaning of "all" ? adding a new record mode ?).

pakal avatar Oct 25 '17 09:10 pakal

@kevin1024 I think that we should just go ahead and make it so that the behaviors of the various keywords make sense. I think we might need to do a major version bump in that case, but this has always kind of annoyed me.

colonelpanic8 avatar Oct 25 '17 10:10 colonelpanic8

What does the Ruby VCR do in this case? My original intent was for the record modes to function the same way in case people come over from Ruby.

kevin1024 avatar Oct 25 '17 16:10 kevin1024

The "all" in VCR Ruby is documented as an overriding of current cassette, if I understand correctly ; https://relishapp.com/vcr/vcr/v/1-3-2/docs/record-modes/all-record-mode

So I guess it'd make sense to switch to that semantic, breaking the compatibility with the previous "append" semantic (for which I see no use case actually - the first cassette records would still be the one used in replay mode).

Woudl it be OK ?

pakal avatar Oct 27 '17 08:10 pakal

Note : it has to be decided whether the most important is keeping compat' with Ruby or with existing vcrpy users. In the first case we correct the "all" behaviour, in the second case we add a new record mode like "override" ; I prefer the first choice though (we could add an "append" mode to keep this behaviour for those who would need it, but I doubt it is helpful to anyone).

pakal avatar Oct 30 '17 08:10 pakal

Just for the record, I would love "all" to override instead of append.

1ucian0 avatar Aug 11 '18 00:08 1ucian0

I would like to revive this old thread. I started using vcrpy and also didn't expect the current "all" behavior. It seems that the change is trivial, but is there a consensus from the maintainers on the design?

dmfigol avatar Jul 31 '19 17:07 dmfigol

A lot of changes have happened to VCRpy since this ticket was opened. As this ticket has become stale, would you mind closing it if it is no longer needed / relevant?

If I haven't heard anything in a week I'll mark it as closed as we have a lot of old tickets that need to be groomed to make it easier to see what is still relevant.

However if it is still needed, please feel free to re-open or create a new ticket.

Thanks! 🙏

neozenith avatar Jan 05 '20 22:01 neozenith

@neozenith it is still relevant. Up to this day, when I need to modify my vcr config / record new cassettes, I spend significant amount of time trying to workaround this issue. Would it be possible to please change 'all' to rerecord?

dmfigol avatar Feb 06 '20 00:02 dmfigol

Thanks for the response @dmfigol!

Also I just wanted to update active PRs and issues to let you know I will no longer be an active maintainer on this project.

I recently changed jobs where I no longer use VCRpy or Python at all so I don't have the bandwidth anymore. I won't be monitoring issues or PRs either.

I will have to delegate to other maintainers or ask for some contributors to step up.

I know the Azure CLI uses VCRpy so it would be nice if they could spare some sprint time to maintain this project as it was invaluable in our CI when I was using it.

Kind regards, Josh

neozenith avatar Feb 15 '20 02:02 neozenith

Folks, FYI, rerecord mode is implemented in a pytest plugin to VCR-py.

https://github.com/kiwicom/pytest-recording#rewrite-record-mode

Hope it might be helpful for you

Stranger6667 avatar Apr 19 '20 09:04 Stranger6667

awesome! Adding a link to the README.

kevin1024 avatar Apr 19 '20 13:04 kevin1024

If you're like me and came here for a workaround until the issue is addressed, here's the simplest one.

When the record mode is all, ignore all previously recorded content.

if vcr.record_mode == "all":
    vcr.persister.load_cassette = lambda cassette_path, serializer: ([], [])

Alternatively, a more verbose but more complete approach.

import vcr
from vcr.persisters.filesystem import FilesystemPersister


class RewriteFilesystemPersister(FilesystemPersister):

    @classmethod
    def load_cassette(cls, cassette_path, serializer):
        return [], []


class VCR(vcr.VCR):

    def __init__(self, **kwargs):
        super().__init__(**kwargs)
        self.adjust_persister()

    def set_record_mode(self, record_mode):
        """Set record mode and adjust persister accordingly."""
        self.record_mode = record_mode
        self.adjust_persister()

    def adjust_persister(self):
        """Helper method to adjust persister depending on the record mode."""
        if self.record_mode == "all":
            self.register_persister(RewriteFilesystemPersister)
        else:
            self.register_persister(FilesystemPersister)

imankulov avatar Sep 08 '21 16:09 imankulov