scrapyd icon indicating copy to clipboard operation
scrapyd copied to clipboard

a new built-in ui

Open mojtabaasadi opened this issue 5 years ago • 15 comments

user interface with it can be parallelly developed has some advantage in monitoring logs and is as simple as current web services

mojtabaasadi avatar Jan 07 '19 09:01 mojtabaasadi

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 of scrapyd'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 the scrapy 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 make scrapyd'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 avatar Jan 07 '19 12:01 cathalgarvey

@cathalgarvey Try ScrapydWeb: https://github.com/my8100/scrapydweb

my8100 avatar Jan 07 '19 13:01 my8100

@my8100 that's a good one . but it has the need of lunching another application

mojtabaasadi avatar Jan 08 '19 05:01 mojtabaasadi

@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?

mojtabaasadi avatar Jan 08 '19 06:01 mojtabaasadi

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.

Digenis avatar Apr 19 '19 13:04 Digenis

Codecov Report

Merging #304 into master will increase coverage by 0.62%. The diff coverage is 100%.

Impacted file tree graph

@@           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[bot] avatar Apr 25 '19 16:04 codecov[bot]

Codecov Report

Merging #304 into master will increase coverage by 0.62%. The diff coverage is 100%.

Impacted file tree graph

@@           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.

codecov[bot] avatar Apr 25 '19 16:04 codecov[bot]

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.

mojtabaasadi avatar Apr 25 '19 16:04 mojtabaasadi

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.

Digenis avatar Apr 26 '19 09:04 Digenis

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?

pawelmhm avatar Nov 23 '21 17:11 pawelmhm

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.

pawelmhm avatar Dec 10 '21 14:12 pawelmhm

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

image

pawelmhm avatar Dec 10 '21 14:12 pawelmhm

@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 avatar Dec 14 '21 09:12 mojtabaasadi

@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.

pawelmhm avatar Dec 14 '21 09:12 pawelmhm

@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!

jpmckinney avatar Dec 20 '21 21:12 jpmckinney