scrapyd
scrapyd copied to clipboard
a new built-in ui
user interface with it can be parallelly developed has some advantage in monitoring logs and is as simple as current web services
NB: Tox is failing because Tox appears to no longer support Py3.3.
@mojtabaasadi: I'm impressed that you've written a web UI for scrapyd
, though I haven't had a chance to try it out. So, that's cool. But, I don't think that we should merge a change that switches scrapyd
to use the UI by default, for several reasons:
- Doing so would effectively make the react project part of
scrapyd
and therefore part ofscrapyd
's maintenance burden -scrapyd
would therefore become a two-language, two-framework project, making it much harder for us to responsibly maintain. In theory we could adopt the UI project into thescrapy
project and maintain it separately, but that would still leave the below issues: - The UI being shipped from a CDN creates an immediate privacy and security problem, and adds an additional point of failure to the project. It also appears to be shipped as a minified webpack file that dynamically loads the UI from Github? Which might be too unclear for users.
- Related: The UI shipping from a personal CDN account would concern many users, for privacy and security reasons. I think organisations using
scrapyd
would be concerned about this especially. - Related: The UI seems to give comments or feedback to users containing bitly tracking links, which is a privacy threat to users.
- The UI being on-by-default would, if I understand React correctly, require an additional service to be run next to
scrapyd
to handle the UI, whether or not the UI is used. For many deployments this might makescrapyd
's process(es) larger than initially budgeted, and require more hardware or manual disabling of the UI.
All of the above said.. I think a UI for scrapyd
would be really cool, so I'd like to help this become a better-supported use for scrapyd
. As an alternative, I'd suggest a change that lets users choose a UI to launch instead of the default: what do you think?
@cathalgarvey Try ScrapydWeb: https://github.com/my8100/scrapydweb
@my8100 that's a good one . but it has the need of lunching another application
@cathalgarvey , I can agree with you for some of reasons . the reason of creating react app was that scrapyd already has a built-in web ui but it is poor . so I thought neither the same should be developed or create standalone single page JavaScript app .that can be developed independently so I went for the second. that's a good idea ( giving option to user) , but how good you think it is to use the same react app for alternative option?
I wouldn't have said it any better than @cathalgarvey.
Regarding this issue as a pull request:
Because of the CDN, I assume this PR is a showcase.
But using a cdn to deliver javascript opens up to XSS
and scrapyd does execute any code uploaded to it.
XSS + scrapyd is just a more complicated curl | bash
Screenshots are a much simpler way to safely showcase the ui.
I'd suggest a change that lets users choose a UI to launch instead of the default: what do you think?
Let's make it easier for users to replace the built-in website with their own.
Codecov Report
Merging #304 into master will increase coverage by
0.62%
. The diff coverage is100%
.
@@ Coverage Diff @@
## master #304 +/- ##
========================================
+ Coverage 68.37% 69% +0.62%
========================================
Files 16 16
Lines 819 813 -6
Branches 96 94 -2
========================================
+ Hits 560 561 +1
+ Misses 230 223 -7
Partials 29 29
Impacted Files | Coverage Δ | |
---|---|---|
scrapyd/website.py | 64.77% <100%> (+5.19%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 1713747...b7da752. Read the comment docs.
Codecov Report
Merging #304 into master will increase coverage by
0.62%
. The diff coverage is100%
.
@@ Coverage Diff @@
## master #304 +/- ##
========================================
+ Coverage 68.37% 69% +0.62%
========================================
Files 16 16
Lines 819 813 -6
Branches 96 94 -2
========================================
+ Hits 560 561 +1
+ Misses 230 223 -7
Partials 29 29
Impacted Files | Coverage Δ | |
---|---|---|
scrapyd/website.py | 64.77% <100%> (+5.19%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 1713747...a167dfa. Read the comment docs.
last commit makes users bale to have their own views base routes , users also can build javascript apps and have routing in it . I used twisted.web.template as template engine , it isn't as powerful as common template engines but it useful at this stage. but documentation is needed in the case of acceptance.
I never heard of twisted.web.template. It's really nice that you found a way to refactor the website without adding new package dependencies.
The idea behind the second commit is something I'd definitely like to see in scrapyd. I'll look it in depth and add more comments.
This is really nice work, but now has conflicts with master and also need to add some tests. It will require work integrating with master. I'll put it on roadmap for 1.4, we could also maybe use some open source css framework? Just add it to project with proper license and link from html here?
Tests are failing because they need to be updated, change this
content = scrapyd_site.children[b'jobs'].render(txrequest)
to
content = scrapyd_site.children[b'jobs'].render_GET(txrequest)
and need to bring back content-type and content-length, same for render_home test.
I would use a separate template for "<head>"
section of HTML document, and we can add some common css stylesheet there for both jobs and home. I played around in couple of minutes with Foundation, and created something liket this, is it less ugly then default? It is just an example, tables in jobs can be given better design etc. cc @jpmckinney what do you think about this PR and idea of adding some css framework to make webservice HTML little less ugly
@pawelmhm thanks for review, the initial idea behind this pr was create nicer UI for scrapyd. but here @cathalgarvey suggested to implement feature that make user able to choose a ui. later commits are trying to achieve so. I will resolve problems and add tests as soon as i can.
@mojtabaasadi I think we can just create default basic bit better UI with some open source CSS, like Twitter Bootstrap or Foundation, but your PR doesn't have to do this. It can be done separately on other branch, on other pull request. Just refactoring website html into templates is a useful thing, so you can just fix tests that are there, add some tests if you think you need it and we can merge.
@mojtabaasadi I think we can just create default basic bit better UI with some open source CSS, like Twitter Bootstrap or Foundation, but your PR doesn't have to do this. It can be done separately on other branch, on other pull request. Just refactoring website html into templates is a useful thing, so you can just fix tests that are there, add some tests if you think you need it and we can merge.
Sounds good to me, agreed!