Skip to content
This repository was archived by the owner on Jul 19, 2018. It is now read-only.

Conversation

@nyov
Copy link
Contributor

@nyov nyov commented Aug 19, 2015

Since f347493, because scrapy.loader does not exist prior.

@nyov
Copy link
Contributor Author

nyov commented Aug 19, 2015

Well that is awkward. I wasn't planning on fixing tests.

@redapple redapple mentioned this pull request Feb 15, 2016
@nyov
Copy link
Contributor Author

nyov commented Mar 4, 2016

Rebased on top of #71
Added some things to try fix bsddb3 bytes keys. Test succeeds now but something looks strange (values converted to bytes on return).

assert mw.db.items() == [('test_key_1', 'test_v_1'),
('test_key_2', 'test_v_2')]
assert mw.db.items() == [(b'test_key_1', b'test_v_1'),
(b'test_key_2', b'test_v_2')]
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 is strange: test_v_1 values are returned as bytes. But were inserted as strings (db.put(b'test_key_1', 'test_v_1')). Not sure if this is bsddb3 not casting back to int/string on fetch or what.

requirements.txt Outdated
@@ -1,5 +1,6 @@
six
boto
hubstorage
Copy link
Contributor

Choose a reason for hiding this comment

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

let's require hubstorage>=0.23

@redapple
Copy link
Contributor

@nyov , I re-triggered the builds on Travis.
All green now

'Programming Language :: Python'
],
install_requires=['Scrapy>=0.22.0']
install_requires=['Scrapy>=1.0.0']
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it require same scrapy version as in requirements.txt?

Copy link
Contributor

Choose a reason for hiding this comment

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

... maybe not, so as to support Scrapy 1.0 users

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure. Perhaps the line can be dropped completely?

@nyov
Copy link
Contributor Author

nyov commented Jun 15, 2016

Updated and rebased on master.
(This includes/outdates the https://github.com/scrapinghub/scrapylib/tree/py3-porting branch)

@nyov nyov changed the title Depends on Scrapy 1.0 Python3 support, Scrapy 1.0+ dependencies Jun 15, 2016
redapple added a commit to scrapy-plugins/scrapy-magicfields that referenced this pull request Jun 29, 2016
redapple added a commit to scrapy-plugins/scrapy-querycleaner that referenced this pull request Jun 30, 2016
@nyov
Copy link
Contributor Author

nyov commented Jul 15, 2016

So I guess this commit won't make it...?

I've just been staring at a new deployment error

  File "/usr/local/lib/python3.5/dist-packages/scrapylib/processors/__init__.py", line 5, in <module>
    from urlparse import urljoin
ImportError: No module named 'urlparse'

And I had this deja-vu (didn't I fix this already?)...

It's really great to see the new packages coming, but I would think a transitional package version would be great as well. Pull in those new depends and strip the package piecemeal.
(Or where did those scrapylib.processors end up?)

@redapple
Copy link
Contributor

@nyov , you're right, a new release of scrapylib with new depends makes sense.
The migration effort to scrapy-plugins has stalled right now and scrapylib.processors is still only in scrapylib

@nyov
Copy link
Contributor Author

nyov commented Jul 15, 2016

Thanks for the info, @redapple.

Should processors simply be moved to scrapy proper, perhaps?
If it ever comes to that¹ , I think it might make sense to package up ItemLoaders and their processors together into a package.
Creating a package for these few processors, but keeping the rest in scrapy doesn't seem sensible.

(¹ IIRC ItemLoader was kept from contrib rather than packaged itself, after several(?) discussions.)

@redapple redapple merged commit 791afe5 into scrapinghub:master Jul 19, 2016
@redapple
Copy link
Contributor

Thanks a ton @nyov .
The switch to scrapy-plugins continues in #76

@nyov nyov deleted the requirements branch August 26, 2016 17:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants