Skip to content
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

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

Open
colonelpanic8 opened this issue Sep 13, 2015 · 30 comments
Labels
core issue related to the core of the library enhancement

Comments

@colonelpanic8
Copy link
Collaborator

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

@kevin1024
Copy link
Owner

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.

@agriffis
Copy link
Contributor

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

@colonelpanic8
Copy link
Collaborator Author

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

@agriffis
Copy link
Contributor

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

@colonelpanic8
Copy link
Collaborator Author

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

self.record_mode != 'all' and \

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.

@agriffis
Copy link
Contributor

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.

@colonelpanic8
Copy link
Collaborator Author

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.

@kevin1024
Copy link
Owner

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.

👍 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?

@hellmanj
Copy link

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?

@colonelpanic8
Copy link
Collaborator Author

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 colonelpanic8 changed the title 'rerecord' mode that essentially ignores the recorded cassette 'rerecord' mode that essentially ignores the recorded cassette, strict validation of record modes Jan 11, 2016
@hellmanj
Copy link

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.

@colonelpanic8
Copy link
Collaborator Author

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.

@hellmanj
Copy link

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
Copy link

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

@colonelpanic8
Copy link
Collaborator Author

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).

@silencev
Copy link

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

@pakal
Copy link

pakal commented Oct 25, 2017

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 ?).

@colonelpanic8
Copy link
Collaborator Author

@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.

@kevin1024
Copy link
Owner

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.

@pakal
Copy link

pakal commented Oct 27, 2017

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
Copy link

pakal commented Oct 30, 2017

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).

@1ucian0
Copy link

1ucian0 commented Aug 11, 2018

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

@dmfigol
Copy link

dmfigol commented Jul 31, 2019

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?

@neozenith neozenith added the stale Issues and PRs that have had birthdays since last activity. Please feel free to reopen though! label Jan 5, 2020
@neozenith
Copy link
Collaborator

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! 🙏

@dmfigol
Copy link

dmfigol commented Feb 6, 2020

@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?

@neozenith neozenith removed the stale Issues and PRs that have had birthdays since last activity. Please feel free to reopen though! label Feb 15, 2020
@neozenith neozenith added core issue related to the core of the library enhancement labels Feb 15, 2020
@neozenith
Copy link
Collaborator

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

@Stranger6667
Copy link
Contributor

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

@kevin1024
Copy link
Owner

awesome! Adding a link to the README.

@imankulov
Copy link

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)

@WilliamDEdwards
Copy link

This bit me. From the documentation, I expected all to overwrite my existing cassettes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core issue related to the core of the library enhancement
Projects
None yet
Development

No branches or pull requests