scrapy / scrapyd

A service daemon to run Scrapy spiders
https://scrapyd.readthedocs.io/en/stable/
BSD 3-Clause "New" or "Revised" License
2.96k stars 569 forks source link

listspiders.json and listversions.json not working when version schemes are mixed #212

Closed jxltom closed 3 months ago

jxltom commented 7 years ago

If one uploads a project with default version schema which is timestamp using scrapyd-client, and then upload a new version of same project but with GIT version which is specified by version = GIT in scrapy.cfg, listspiders.json and listversions.json won't working.

For listspiders.json, this error is raised.

 File \"/usr/local/lib/python3.4/runpy.py\", line 170, in _run_module_as_main
 \"__main__\", mod_spec)
 File \"/usr/local/lib/python3.4/runpy.py\", line 85, in _run_code
 exec(code, run_globals)
 File \"/usr/local/lib/python3.4/site-packages/scrapyd/runner.py\", line 40, in <module>
 main()
 File \"/usr/local/lib/python3.4/site-packages/scrapyd/runner.py\", line 35, in main
 with project_environment(project):
 File \"/usr/local/lib/python3.4/contextlib.py\", line 59, in __enter__
 return next(self.gen)
 File \"/usr/local/lib/python3.4/site-packages/scrapyd/runner.py\", line 16, in project_environment
 version, eggfile = eggstorage.get(project, eggversion)
 File \"/usr/local/lib/python3.4/site-packages/scrapyd/eggstorage.py\", line 28, in get
 version = self.list(project)[-1]
 File \"/usr/local/lib/python3.4/site-packages/scrapyd/eggstorage.py\", line 37, in list
 return sorted(versions, key=LooseVersion)
 File \"/usr/local/lib/python3.4/distutils/version.py\", line 58, in __lt__
 c = self._cmp(other)
 File \"/usr/local/lib/python3.4/distutils/version.py\", line 343, in _cmp
 if self.version < other.version:
TypeError: unorderable types: int() < str()

For listversions.json, this error is raised.

unorderable types: int() < str()
Digenis commented 7 years ago

Indeed, it looks like mixing git and timestamp versioning schemes can't work together.

Even if the timestamp is stored as a string, the comparisons will eventually be wrong.

We should note that in the doc and update the addversion ws to make it reject new deployments that change the version scheme. That's all for milestone 1.2

Can you use the delversion or delproject webservices for this project while the version sorting is broken?

jxltom commented 7 years ago

Yes, these delversion or delproject are working well. There is no problem in deleting. Only listspiders and listversions are affected by this bug.

Digenis commented 7 years ago

Ok, this means that nothing special needs to be added to the fix I describe.

Thank you for reporting this, I will write a patch when I have the time.

The solution for broken projects is exactly what you tried, use delproject (or delversion) to clean up versions with a different versioning scheme.

jxltom commented 7 years ago

Yes, it will be nice if there is a clean-up when different versioning schemes are used.

Digenis commented 7 years ago

@jxltom, although I can't reproduce your issue I'm sure it's https://bugs.python.org/issue14894 It seems that when python3 dropped int<>str comparisons, LooseVersion wasn't updated.

We shouldn't fix this issue by writing a workaround for a python bug. What is wrong in our side is allowing changing version schemes without purging previous deployments.

But even if scrapyd and scrapyd-client had a way to exchange the version schemes before deploying, when LooseVersion parses different branch names or annotated tags it splits them using the regex (\d+|[a-z]+|\.) so it generates different versioning schemes whose components will also not always have comparable types.

The above is something I want to reproduce but I can't. (probably for the same reason I can't reproduce the original issue)

We need more ideas and more input. I'd rather avoid letting an experimental solution in 1.2. and since this is not a critical issue, it can skip this release.

Digenis commented 7 years ago

when LooseVersion parses different branch names or annotated tags it splits them using the regex (\d+|[a-z]+|.) so it generates different versioning schemes whose components will also not always have comparable types.

After fixing my virtualenv I managed to reproduce this in python 3 so the conclusion remains the same:

We need more ideas and more input. I'd rather avoid letting an experimental solution in 1.2. and since this is not a critical issue, it can skip this release.

jpmckinney commented 3 years ago

I'm not sure why this is labelled wontfix. It is easy to reproduce, and even if the bug is upstream, we should try to fix it.

jpmckinney commented 3 months ago

distutils.LooseVersion is gone without a replacement so it was replaced with packaging.version.Version #469 which does not have this issue.