Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upRun crawl (of WARC) tests on travis #3
Conversation
ikreymer
added some commits
Apr 9, 2019
ikreymer
requested a review
from
N0taN3rd
Apr 9, 2019
ikreymer
added some commits
Apr 10, 2019
N0taN3rd
requested changes
Apr 11, 2019
| better_exceptions | ||
| cchardet |
This comment has been minimized.
This comment has been minimized.
N0taN3rd
Apr 11, 2019
Member
cchardet (for aiohttp), pandantic, and starlette (we directly import these) should stay.
This comment has been minimized.
This comment has been minimized.
ikreymer
Apr 13, 2019
Author
Member
Adding pydantic and starlette actually breaks the install, because the latest versions aren't compatible with latest fastapi. We don't want the latest version of these or even to worry about which version to use, but rather use whichever version is installed when fastapi is installed.
| @@ -11,6 +11,9 @@ | |||
| from starlette.exceptions import HTTPException | |||
| from ujson import dumps as ujson_dumps | |||
|
|
|||
| import logging | |||
This comment has been minimized.
This comment has been minimized.
N0taN3rd
Apr 11, 2019
•
Member
is there any reason why we are jacking uvicorns logger rather than using our own?
Also we should keep all imports sorted like below and move the logger variable below all
| import logging | |
| from __future__ import annotations | |
| import logging | |
| import uuid | |
| from asyncio import AbstractEventLoop, gather as aio_gather, get_event_loop | |
| from functools import partial | |
| from typing import Dict, List, Optional, Union | |
| from urllib.parse import urlsplit | |
| from aiohttp import AsyncResolver, ClientSession, TCPConnector | |
| from aioredis import Redis | |
| from starlette.exceptions import HTTPException | |
| from ujson import dumps as ujson_dumps | |
| from .schema import CrawlInfo, CreateCrawlRequest, StartCrawlRequest | |
| from .utils import env, init_redis | |
| __all__ = ['Crawl', 'CrawlManager'] | |
| logger = logging.getLogger('uvicorn') |
| @@ -10,8 +10,7 @@ crawlmanager | |||
|
|
|||
| To use crawlmanager, run: | |||
| ```bash | |||
| poetry install | |||
| # or "pip install -r reqs/requirements.txt -r reqs/dev-requirements.txt" | |||
| python setup.py install | |||
This comment has been minimized.
This comment has been minimized.
N0taN3rd
Apr 11, 2019
Member
setup.py is not required unless distributing or installing this project globally
rather should be pip install -r requirements since these directions are for running within the repo
| python setup.py install | |
| pip install -r requirements.txt -r dev-requirements.txt |
| @@ -229,7 +229,7 @@ venv.bak/ | |||
| localCompose | |||
| pip-wheel-metadata | |||
| scripts | |||
| tests | |||
| #tests | |||
This comment has been minimized.
This comment has been minimized.
N0taN3rd
Apr 11, 2019
Member
I think it would be best to not include the tests in the docker container.
If we do need them it should be a separate dev container
ikreymer
added some commits
Apr 13, 2019
ikreymer commentedApr 9, 2019
No description provided.