♻️ Refactor FastCrawler core codebase and synced to FastAPI backend client #53
Conversation
ManiMozaffar
changed the title
ManiMozaffar
changed the title
ManiMozaffar
changed the title
| from .process import Process | ||
|
|
||
|
|
||
| def list_process(crawlers: list[Process] | Process) -> list[Process]: |
There was a problem hiding this comment.
bad naming
bad design
There was a problem hiding this comment.
Will be fixed in next PR.
|
|
||
| @property | ||
| def get_all_serves(self) -> list[Callable]: | ||
| def get_all_serves(self) -> list[Coroutine[Any, Any, None]]: |
There was a problem hiding this comment.
What type I should use then?
I have to look up in uvicorn to check the protocol, then define the servable coroutine.
| self.task = Task( | ||
| start_cond=cond or "every 1 second", | ||
| name=spider.__class__.__name__ + str(uuid4()), | ||
| name=f"{uuid4()}@{spider.__class__.__name__}", |
There was a problem hiding this comment.
this needs lots of comments and examples
There was a problem hiding this comment.
Will add doc src in future.
|
|
||
| @property | ||
| def is_stopped(self) -> set: | ||
| def instances(self) -> list[Self | "Spider"]: |
There was a problem hiding this comment.
wrong encapsulation
There was a problem hiding this comment.
True, will change that in next PR
| self._instances = [self] | ||
| return self._instances | ||
|
|
||
| @instances.setter |
There was a problem hiding this comment.
what do you mean? the instances?
any suggestion?
Like linked_spiders?
There was a problem hiding this comment.
spiders maybe? i don't know you decide
| if response | ||
| ] | ||
| await self.save(results) | ||
| for idx in range(0, len(urls), self.get_batch_size): |
There was a problem hiding this comment.
indeed, I've wrote to you in past that I don't like this part of code.
All would goo to Batch class.
I haven't prototyped it yet.
Currently vahid would be working on core, with that new feature that we discussed yesterday.
Then i'll kick off implementing the Batch class.
| ] | ||
| ) | ||
| return {url: result for url, result in zip(tasks.keys(), results)} | ||
| tasks = [] |
There was a problem hiding this comment.
use list() instead of []
There was a problem hiding this comment.
that's not correct. using language construct is always better.
Linters prefer language construct
and also it's better in term of perfomance
| port: int | ||
| username: str | None = None | ||
| password: str | None = None | ||
| protocol: str = "http://" |
There was a problem hiding this comment.
this should be an enum
There was a problem hiding this comment.
will be done in another pr.
btw, what are the possibilities? I should research.
|
|
||
|
|
||
| @dataclass | ||
| class RequestCycle: |
There was a problem hiding this comment.
Request and Response are easy conecepts
but anything Cycle related needs lots of doc strings and comments and examples
There was a problem hiding this comment.
sure, will do. in another PR
| urls_1 = [Request(url=url) for url in (["http://127.0.0.1:8000/throttled/3/"] * 2)] | ||
| urls_2 = [Request(url=url) for url in (["http://127.0.0.1:8000/throttled/5/"] * 1)] | ||
| urls = [ | ||
| Request(url="http://127.0.0.1:8000/throttled", data={"seconds": 0.01}), |
There was a problem hiding this comment.
you need a variable for this url
There was a problem hiding this comment.
Will be fix in next PR
No description provided.