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

add new AnyResponse #195

Merged
merged 8 commits into from
Jan 18, 2024
Merged

add new AnyResponse #195

merged 8 commits into from
Jan 18, 2024

Conversation

BurnzZ
Copy link
Member

@BurnzZ BurnzZ commented Jan 16, 2024

A way to address zytedata/zyte-spider-templates#25.

Related PRs:

Not quite sure if we should have a detailed documentation about this class in the tutorials since it may be deprecated in the next few months when Zyte API handles Auto-Configuration (i.e. selecting the best extraction source between HttpResponse and BrowserResponse). EDIT: Added docs.

Copy link

codecov bot commented Jan 16, 2024

Codecov Report

Merging #195 (27b594e) into master (d783365) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #195      +/-   ##
==========================================
+ Coverage   98.44%   98.46%   +0.02%     
==========================================
  Files          30       31       +1     
  Lines        1414     1437      +23     
==========================================
+ Hits         1392     1415      +23     
  Misses         22       22              
Files Coverage Δ
web_poet/__init__.py 100.00% <ø> (ø)
web_poet/page_inputs/__init__.py 100.00% <100.00%> (ø)
web_poet/page_inputs/response.py 100.00% <100.00%> (ø)

@kmike
Copy link
Member

kmike commented Jan 16, 2024

Not quite sure if we should have a detailed documentation about this class in the tutorials since it may be deprecated in the next few months when Zyte API handles Auto-Configuration (i.e. selecting the best extraction source between HttpResponse and BrowserResponse).

I think we need to have docs for this class; I'm not sure it makes sense to deprecate it later. It shouldn't be only about the extraction source, it should be possible to use this dependency without any extraction as well. The logic is the following: if there is BrowserResponse dependency in the plan, BrowserResponse is going to be used; if not, it probably should use HttpResponse.

@BurnzZ BurnzZ marked this pull request as ready for review January 17, 2024 06:36
@BurnzZ BurnzZ changed the title add new HttpOrBrowserResponse add new AnyResponse Jan 17, 2024
@kmike kmike merged commit b7c6d51 into master Jan 18, 2024
16 checks passed
@kmike
Copy link
Member

kmike commented Jan 18, 2024

Looks good, thanks @BurnzZ!

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

Successfully merging this pull request may close these issues.

4 participants