Skip to content

httpx from scratch #1823

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ authors = [{ name = "Replicate", email = "[email protected]" }]
license.file = "LICENSE"
urls."Source" = "https://github.com/replicate/cog"

requires-python = ">=3.7"
requires-python = ">=3.8"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this officially drop 3.7?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah

dependencies = [
# intentionally loose. perhaps these should be vendored to not collide with user code?
"attrs>=20.1,<24",
"fastapi>=0.75.2,<0.99.0",
# we may not need http2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We almost certainly don't. What's the cost of including it vs not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the only real motivation for this when it was written is using http2 for file uploads which did not go through localhost at the time. h2 is not a big library so I don't think this is terribly consequential either way

"httpx[http2]>=0.21.0,<1",
"pydantic>=1.9,<2",
"PyYAML",
"requests>=2,<3",
Expand All @@ -27,14 +29,15 @@ dependencies = [
optional-dependencies = { "dev" = [
"black",
"build",
"httpx",
'hypothesis<6.80.0; python_version < "3.8"',
'hypothesis; python_version >= "3.8"',
"respx",
'numpy<1.22.0; python_version < "3.8"',
'numpy; python_version >= "3.8"',
"pillow",
"pyright==1.1.347",
"pytest",
"pytest-asyncio",
"pytest-httpserver",
"pytest-rerunfailures",
"pytest-xdist",
Expand Down
86 changes: 0 additions & 86 deletions python/cog/files.py

This file was deleted.

26 changes: 1 addition & 25 deletions python/cog/json.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import io
from datetime import datetime
from enum import Enum
from types import GeneratorType
from typing import Any, Callable
from typing import Any

from pydantic import BaseModel

from .types import Path


def make_encodeable(obj: Any) -> Any:
"""
Expand Down Expand Up @@ -39,24 +36,3 @@ def make_encodeable(obj: Any) -> Any:
if isinstance(obj, np.ndarray):
return obj.tolist()
return obj


def upload_files(obj: Any, upload_file: Callable[[io.IOBase], str]) -> Any:
"""
Iterates through an object from make_encodeable and uploads any files.

When a file is encountered, it will be passed to upload_file. Any paths will be opened and converted to files.
"""
# skip four isinstance checks for fast text models
if type(obj) == str: # noqa: E721
return obj
if isinstance(obj, dict):
return {key: upload_files(value, upload_file) for key, value in obj.items()}
if isinstance(obj, list):
return [upload_files(value, upload_file) for value in obj]
if isinstance(obj, Path):
with obj.open("rb") as f:
return upload_file(f)
if isinstance(obj, io.IOBase):
return upload_file(obj)
return obj
1 change: 0 additions & 1 deletion python/cog/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,3 @@ def setup_logging(*, log_level: int = logging.NOTSET) -> None:

# Reconfigure log levels for some overly chatty libraries
logging.getLogger("uvicorn.access").setLevel(logging.WARNING)
logging.getLogger("urllib3.connectionpool").setLevel(logging.ERROR)
42 changes: 23 additions & 19 deletions python/cog/predictor.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,34 +22,24 @@
)
from unittest.mock import patch

import structlog

import cog.code_xforms as code_xforms

try:
from typing import get_args, get_origin
except ImportError: # Python < 3.8
from typing_compat import get_args, get_origin # type: ignore

import structlog
import yaml
from pydantic import BaseModel, Field, create_model
from pydantic.fields import FieldInfo

# Added in Python 3.9. Can be from typing if we drop support for <3.9
from typing_extensions import Annotated

from . import code_xforms
from .errors import ConfigDoesNotExist, PredictorNotSet
from .types import (
CogConfig,
Input,
URLPath,
)
from .types import (
File as CogFile,
)
from .types import (
Path as CogPath,
)
from .types import CogConfig, Input, URLTempFile
from .types import File as CogFile
from .types import Path as CogPath
from .types import Secret as CogSecret

log = structlog.get_logger("cog.server.predictor")
Expand Down Expand Up @@ -89,14 +79,20 @@ def run_setup(predictor: BasePredictor) -> None:
return

weights: Union[io.IOBase, Path, str, None]

weights_url = os.environ.get("COG_WEIGHTS")
# this is the source of some bugs
# https://github.com/replicate/cog-sdxl/blob/main/predict.py#L184-L185
# https://github.com/replicate/cog-llama-template/blob/main/predict.py#L44-L46
weights_path = "weights"

# TODO: Cog{File,Path}.validate(...) methods accept either "real"
# paths/files or URLs to those things. In future we can probably tidy this
# up a little bit.
# TODO: CogFile/CogPath should have subclasses for each of the subtypes

# this is a breaking change
# previously, CogPath wouldn't be converted in setup(); now it is
# essentially everyone needs to switch from Path to str (or a new URL type)
if weights_url:
if weights_type == CogFile:
weights = cast(CogFile, CogFile.validate(weights_url))
Expand Down Expand Up @@ -266,12 +262,20 @@ def cleanup(self) -> None:
Cleanup any temporary files created by the input.
"""
for _, value in self:
# Handle URLPath objects specially for cleanup.
# Handle URLTempFile objects specially for cleanup.
# Also handle pathlib.Path objects, which cog.Path is a subclass of.
# A pathlib.Path object shouldn't make its way here,
# but both have an unlink() method, so we may as well be safe.
if isinstance(value, (URLPath, Path)):
value.unlink(missing_ok=True)
if isinstance(value, (URLTempFile, Path)):
try:
value.unlink(missing_ok=True)
except FileNotFoundError:
pass

# if we had a separate method to traverse the input and apply some function to each value
# we could have cleanup/get_tempfile/convert functions that operate on a single value
# and get recursively applied to any nested part of the input.
# unlike cleanup, convert is supposed to mutate though, so it's tricky


def validate_input_type(type: Type[Any], name: str) -> None:
Expand Down
Loading
Loading