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

Remove the experimental name space from cookie parameters #144

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
@@ -1,6 +1,27 @@
Changes
=======

To be released
--------------

* Cookie support is no longer experimental:

* If the ``COOKIES_ENABLED`` setting is ``True`` (default), automatic request
parameter mapping now sets ``responseCookies`` to ``True`` and maps request
cookies to ``requestCookies``.
Comment on lines +9 to +11
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that, by default, all Zyte API requests with automap will start including "responseCookies": True among their parameters. Similar to how we do with httpResponseHeaders, but in this case the behavior also affects browser rendering and automatic extraction scenarios. The test expectation updates are a great way to get an idea of the impact.

This is a big one, the reason I went for 0.13.0 instead of 0.12.3, and I wonder whether or not this is the right call. I wonder if we should implement an opt-in setting for responseCookies, or make it so that it is only added to requests if requestCookies is also added (either manually by the user our automatically mapped).


* The ``ZYTE_API_EXPERIMENTAL_COOKIES_ENABLED`` setting is now deprecated.
When enabled, however, the ``experimental`` name space is still used for
automatic request parameter mapping.

* If you use ``requestCookies``, ``responseCookies``, or ``cookieManagement``
within the ``experimental`` name space in request parameters, a deprecation
warning is now logged.

* The ``responseCookies`` response parameter is now handled the same as
``experimental.responseCookies``; the latter still works but is deprecated.


0.14.1 (2024-01-17)
-------------------

Expand All @@ -18,6 +39,7 @@ Changes
work as expected with
:class:`~scrapy.pqueues.DownloaderAwarePriorityQueue`.


0.14.0 (2024-01-15)
-------------------

Expand All @@ -29,6 +51,7 @@ Changes

* Added support for ``zyte_common_items.JobPosting`` to the scrapy-poet provider.


0.13.0 (2023-12-13)
-------------------

Expand Down Expand Up @@ -71,6 +94,7 @@ Changes

* Test and CI improvements.


0.12.2 (2023-10-19)
-------------------

Expand All @@ -79,6 +103,7 @@ Changes
* When logging Zyte API requests, truncation now uses
"..." instead of Unicode ellipsis.


0.12.1 (2023-09-29)
-------------------

Expand Down Expand Up @@ -106,7 +131,7 @@ Changes
Experimental is treated as a namespace, and its parameters are the ones
counted, i.e. there is no ``scrapy-zyte-api/request_args/experimental`` stat,
but there are stats like
``scrapy-zyte-api/request_args/experimental.responseCookies``.
``scrapy-zyte-api/request_args/experimental.foo``.


0.11.1 (2023-08-25)
Expand Down
13 changes: 6 additions & 7 deletions docs/reference/fingerprint-params.rst
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ fingerprints for Zyte API requests based on the following Zyte API parameters:
:http:`request:httpRequestText` values generate the same signature.

- Output parameters (:http:`request:browserHtml`,
:http:`request:httpResponseBody`, :http:`request:httpResponseHeaders`,
:http:`request:responseCookies`, :http:`request:screenshot`, and
:ref:`automatic extraction outputs <zyte-api-extract-fields>` like
:http:`request:product`)
:http:`request:httpResponseBody`, :http:`request:responseCookies`,
:http:`request:httpResponseHeaders`, :http:`request:responseCookies`,
:http:`request:screenshot`, and :ref:`automatic extraction outputs
<zyte-api-extract-fields>` like :http:`request:product`)

- Rendering option parameters (:http:`request:actions`,
:http:`request:device`, :http:`request:javascript`,
Expand All @@ -42,10 +42,9 @@ The following Zyte API parameters are *not* taken into account for request
fingerprinting:

- Request header parameters (:http:`request:customHttpRequestHeaders`,
:http:`request:requestHeaders`)
:http:`request:requestHeaders`, :http:`request:requestCookies`)

- Request cookie parameters (:http:`request:cookieManagement`,
:http:`request:requestCookies`)
- :http:`request:cookieManagement`

- Session handling parameters (:http:`request:sessionContext`,
:http:`request:sessionContextParameters`)
Expand Down
27 changes: 12 additions & 15 deletions docs/reference/request.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@ Automatic mapping
- :attr:`Request.body <scrapy.http.Request.body>` becomes
:http:`request:httpRequestBody`.

- If :ref:`ZYTE_API_EXPERIMENTAL_COOKIES_ENABLED` is ``True``,
:setting:`COOKIES_ENABLED <scrapy:COOKIES_ENABLED>` is ``True`` (default),
and :attr:`Request.meta <scrapy.http.Request.meta>` does not set
- If the :setting:`COOKIES_ENABLED <scrapy:COOKIES_ENABLED>` is ``True``
(default), and :attr:`Request.meta <scrapy.http.Request.meta>` does not set
:reqmeta:`dont_merge_cookies <scrapy:dont_merge_cookies>` to ``True``:

- :http:`request:experimental.responseCookies` becomes ``True``.
- :http:`request:responseCookies` becomes ``True``.

- Cookies from the :reqmeta:`cookiejar <scrapy:cookiejar>` become
:http:`request:experimental.requestCookies`.
:http:`request:requestCookies`.

All cookies from the cookie jar are set, regardless of their cookie
domain. This is because Zyte API requests may involve requests to
Expand Down Expand Up @@ -115,20 +114,18 @@ following parameters:
"value": "application/json"
}
],
"experimental": {
"requestCookies": [
{
"name": "a",
"value": "b",
"domain": ""
}
],
"responseCookies": true
},
"httpResponseBody": true,
"httpResponseHeaders": true,
"httpRequestBody": "eyJmb28iOiAiYmFyIn0=",
"httpRequestMethod": "POST",
"requestCookies": [
{
"name": "a",
"value": "b",
"domain": ""
}
],
"responseCookies": true,
"url": "https://httpbin.org/anything"
}

Expand Down
7 changes: 3 additions & 4 deletions docs/reference/response.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ Zyte API response parameters are mapped into :ref:`response class
<scrapy_zyte_api.responses.ZyteAPIResponse.status>`.

- :http:`response:httpResponseHeaders` and
:http:`response:experimental.responseCookies` become
:class:`response.headers
:http:`response:responseCookies` become :class:`response.headers
<scrapy_zyte_api.responses.ZyteAPIResponse.headers>`.

- :http:`response:experimental.responseCookies` is also mapped into the
request :reqmeta:`cookiejar <scrapy:cookiejar>`.
- :http:`response:responseCookies` is also mapped into the request
:reqmeta:`cookiejar <scrapy:cookiejar>`.

- :http:`response:browserHtml` and :http:`response:httpResponseBody` are
mapped into both
Expand Down
18 changes: 4 additions & 14 deletions docs/reference/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,6 @@ Default: ``True``
Can be set to ``False`` to disable scrapy-zyte-api.


.. _ZYTE_API_EXPERIMENTAL_COOKIES_ENABLED:

ZYTE_API_EXPERIMENTAL_COOKIES_ENABLED
=====================================

Default: ``False``

See :ref:`request-automatic`.


.. _ZYTE_API_FALLBACK_REQUEST_FINGERPRINTER_CLASS:

ZYTE_API_FALLBACK_REQUEST_FINGERPRINTER_CLASS
Expand Down Expand Up @@ -182,11 +172,11 @@ If the cookies to be set during :ref:`request mapping <request>` exceed this
limit, a warning is logged, and only as many cookies as the limit allows are
set for the target request.

To silence this warning, set :http:`request:experimental.requestCookies`
manually, e.g. to an empty :class:`dict`.
To silence this warning, set :http:`request:requestCookies` manually, e.g. to
an empty :class:`dict`.

Alternatively, if :http:`request:experimental.requestCookies` starts supporting
more than 100 cookies, update this setting accordingly.
Alternatively, if :http:`request:requestCookies` starts supporting more than
100 cookies, update this setting accordingly.


.. _ZYTE_API_MAX_REQUESTS:
Expand Down
4 changes: 1 addition & 3 deletions docs/usage/automap.rst
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,7 @@ following parameters:

{
"browserHtml": true,
"experimental": {
"responseCookies": true
},
"responseCookies": true,
"requestHeaders": {"referer": "https://example.com/"},
"url": "https://quotes.toscrape.com"
}
Expand Down
3 changes: 2 additions & 1 deletion scrapy_zyte_api/_cookies.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ def _process_cookies(
):
if not cookie_jars:
return
response_cookies = api_response.get("experimental", {}).get("responseCookies")
old_response_cookies = api_response.get("experimental", {}).get("responseCookies")
response_cookies = api_response.get("responseCookies", old_response_cookies)
if not response_cookies:
return
cookie_jar = _get_cookie_jar(request, cookie_jars)
Expand Down
Loading