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

Contracts can't be written for callbacks that receive cb_kwargs #3985

Closed
jvani opened this issue Aug 28, 2019 · 2 comments · Fixed by #3988
Closed

Contracts can't be written for callbacks that receive cb_kwargs #3985

jvani opened this issue Aug 28, 2019 · 2 comments · Fixed by #3988
Labels

Comments

@jvani
Copy link

@jvani jvani commented Aug 28, 2019

Description

Contracts can't be written for callbacks that receive cb_kwargs

Steps to Reproduce

  1. Run scrapy check with the following spider:
class CbKwargsContract(scrapy.Spider):
    name = 'cb_kwargs_contract'

    def start_requests(self):
        yield scrapy.Request("https://httpbin.org/get", cb_kwargs={"arg1": "foo"})

    def parse(self, response, arg1):
        """
        @url https://httpbin.org/get
        """
        self.logger.info(arg1)

Expected behavior:
Ideally would be possible to add args to the contract (e.g., @url https://httpbin.org/get foo)

Actual behavior: Fails with a TypeError

Reproduces how often: 100%

Versions

Scrapy 1.7.3

@jvani jvani changed the title Contracts can't be written for callbacks that receive cb_kwargs Contracts can't be written for callbacks that receive cb_kwargs Aug 28, 2019
@elacuesta
Copy link
Member

@elacuesta elacuesta commented Aug 28, 2019

I can reproduce:

E
======================================================================
ERROR: [cb_kwargs_contract] parse (callback)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/.../lib/python3.6/site-packages/scrapy/contracts/__init__.py", line 95, in cb_wrapper
    output = cb(response)
TypeError: parse() missing 1 required positional argument: 'arg1'

----------------------------------------------------------------------
Ran 0 contracts in 0.984s

FAILED (errors=1)

@elacuesta elacuesta added the bug label Aug 28, 2019
@elacuesta
Copy link
Member

@elacuesta elacuesta commented Aug 28, 2019

My first thought was to replace output = cb(response) with output = cb(response, **request.cb_kwargs) in https://github.com/scrapy/scrapy/blob/1.7.3/scrapy/contracts/__init__.py#L95. However, with that change I still get TypeError: parse() missing 1 required positional argument: 'arg1'.

I suspect this is due to the fact that the parsing method is not called as a Deferred callback, and the arguments are interpreted as positional instead of keyword ones. Refactoring the parsing method signature to add a default value for the expected argument (something like arg1=None) does work in that scenario, but I think we need a slightly more complex solution is in order to prevent that restriction.

Scratch that, the request is generated from the contract definition, hence the cb_kwargs attribute is in fact empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants