[add] API to Crawly.Manager #118
Conversation
lib/crawly/engine.ex
Outdated
| def get_manager(spider_name) do | ||
| case Map.fetch(running_spiders(), spider_name) do | ||
| :error -> | ||
| {:error, :spider_non_exist} |
There was a problem hiding this comment.
Basically it means that the spider is not running atm
| {:error, :spider_non_exist} | ||
|
|
||
| {:ok, pid_sup} -> | ||
| Supervisor.which_children(pid_sup) |
There was a problem hiding this comment.
It might not be needed as you already have a spider here pid here.
lib/crawly/manager_api.ex
Outdated
| @@ -0,0 +1,19 @@ | |||
| defmodule Crawly.ManagerAPI do | |||
There was a problem hiding this comment.
I would want to avoid adding extra modules here. Please put add workers code to the manager gen server.
test/features/managet_test_spider.ex
Outdated
| @@ -0,0 +1,48 @@ | |||
| defmodule Features.Manager.TestSpider do | |||
There was a problem hiding this comment.
Isn't it possible to re-use one of already defined test spiders?
There was a problem hiding this comment.
this test_spider was defined inside the ManagerTest. I'm splitting it out to a commonplace so others tests can use it
test/manager_api_test.exs
Outdated
| alias Features.Manager.TestSpider | ||
|
|
||
| setup do | ||
| Application.put_env(:crawly, :concurrent_requests_per_domain, 1) |
There was a problem hiding this comment.
This is not required if you can use the settings override on a spider level.
filipevarjao
force-pushed
the
crawly_manager_api
branch
from
0131448
to
823509f
Compare
|
cool, could you merge this PR?
|
|
Sure! Thanks for the contribution @filipevarjao. I wonder if you still have a bit of time to help me with Crawly, or you've got another assignment already? |
|
happy to help, and I would like to help more, do you still interested to investigate it by the benchmark? or is there a different priority now? let me know |
|
@filipevarjao Yes, I still plan to continue with the benchmark, however now I am a bit limited on time, as we're wrapping up a project. Hope to have a bit more time in 2 weeks time. In any case if you can update the PR for it, it will be helpful. Otherwise I have quite a few open tickets here and in https://github.com/oltarasenko/crawly_ui |
In order to expose the spyder Manager this PR adds the API for Crawly.Manager