Находки в опенсорсе: Python
1.06K subscribers
5 photos
333 links
Легкие задачки в опенсорсе из мира Python

Чат: @opensource_findings_chat
Download Telegram
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Cache the negotiation parts (#1455)


Most of the headers are simple and repeatable. Most users will just send Content-Type: application/json and Accept: application/json like 99% of the time. So, there's no need to run the negotiation process the second time. We already know the final result. So, go through the https://github.com/wemake-services/django-modern-rest/blob/master/dmr/negotiation.py and find places where we can cache header_value -> parser_class or header_value -> renderer_class computations.

Prefer not to have any API changes here.


#good_first_issue #help_wanted #performance #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Pre-compute headers and cookies for `@modify` (#1456)


This is a very important hot-path optimization: currently we build headers and cookies for each response:

django-modern-rest/dmr/validation/response.py

Lines 106 to 110 in 405ba9b

But, as you can see, there's nothing special about each of these calls, they are all the same for the same endpoint. That's why I propose to make this call only once when the endpoint is built in the import time. It would be faster.


#good_first_issue #help_wanted #performance #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Provide concrete views (#1457)


Currently we have a lot of reusable views for auth.
But, this is not really cool to do the manual stuff. Even if it take just several lines. Especially, when I don't need any customizations.

I propose providing new folders near the existing views/, let's call it concrete_views/ and we will put all the same views with default implementations there.

This way people can just import and use them with no manual work.

This would require a bit of integration testing :)
We would need to test default implementations the same way we test reusable views now.
And we should highlight these new views in the docs. And suggest using them over the more complex reusable ones.


#feature #help_wanted #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Support explicitly abstract controllers (#1458)


Currently we only infer Controller.is_abstract ourself:

django-modern-rest/dmr/controller.py

Line 206 in 405ba9b

But, there might be valid cases where people want to define controller to be reused without any customizations or with just the input / return type customizations.

We must respect top-level is_abstract controller definition. Child classes without explicit is_abstract = True must be treated as concrete if they have the serializer.

This way people can create generic controller only on some parts. This would also need a doc update in the "Reusable code" section with the example.


#feature #good_first_issue #help_wanted #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Improve testing of complex type-forms (#1460)


We test quite a lot of type forms in our tests. But, we never test code like:

type UserBody = Body[User]

class MyController(...):
def post(self, parsed_body: UserBody) -> str: ...

This must define the correct component parser for Body. The same should be tested with UserBody: TypeAlias = Body[User]

For all the serializers.

One more thing: union type with ResponseSpecMetadata.

class MyController(...):
def get(self) -> Annotated[UserModel, ResponseSpecMetadata(headers={'X-Id': HeaderSpec()})] | str: ...

I think that there can be bugs in this code, see unwrap_annotation in litestar on how we can properly unwrap all annotations and get the metadata.

Please, add tests first. Then fix bugs if present. Feel free to discuss the design if needed.


#feature #help_wanted #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Make sure that it is possible to customize `operation_id` generation (#1461)


If people need to tailor the operation_id exact look, we need to be able to provide a callback to do that.

I think that we need to do it somewhere here:

django-modern-rest/dmr/openapi/core/context.py

 Line 29 in 405ba9b

django-modern-rest/dmr/openapi/core/context.py

 Line 80 in 405ba9b

Probably via a callback? Other options are also fine :)
But, make sure that the default implementation will be kept.


#feature #good_first_issue #help_wanted #openapi #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Support better schema generation parameters (#1462)


Currently we don't pass quite a lot of options to both pydantic and msgspec schema generation:

django-modern-rest/dmr/plugins/pydantic/schema.py

Lines 27 to 30 in 405ba9b

When pydantic defines this method as:

def json_schema(
self,
*,
by_alias: bool = True,
ref_template: str = DEFAULT_REF_TEMPLATE,
union_format: Literal['any_of', 'primitive_type_array'] = 'any_of',
schema_generator: type[GenerateJsonSchema] = GenerateJsonSchema,
mode: JsonSchemaMode = 'validation',
) -> dict[str, Any]:

The same with msgspec:

django-modern-rest/dmr/plugins/msgspec/schema.py

Lines 24 to 27 in 405ba9b

Signature:

def schema(
type: Any,
*,
schema_hook: Optional[Callable[[type], dict[str, Any]]] = None,
ref_template: str = _REF_TEMPLATE,
) -> dict[str, Any]:

So, what we need to do:

1. Provide way to easily customize the schema generation for both pydantic and msgspec
2. Both would have different API, matching the current defaults
3. Document how to customize the schema generation this way: create your own serializer, subclass schema generator, use it
4. Add tests for 2 of these cases, especially with custom schema_hook and schema_generator options

#feature #good_first_issue #help_wanted #openapi #opensource_september #django_modern_rest
sent via relator
🚀 New issue to ag2ai/faststream by @Lancetnik
📝 Bug: `StreamSub` warns that `no_ack` has no effect with a consumer group, but the flag is forwarded to XREADGROUP NOACK (#3126)


Describe the bug

StreamSub(..., group=..., consumer=..., no_ack=True) emits

RuntimeWarning: `no_ack` has no effect with consumer group

but the flag does have an effect: it reaches Redis as XREADGROUP ... NOACK, so entries never enter the PEL. The warning is a leftover from a time when the flag was not forwarded.

How to reproduce

from faststream.redis import RedisBroker, StreamSub

broker = RedisBroker()


@broker.subscriber(
stream=StreamSub("my-stream", group="my-group", consumer="c1", no_ack=True),
)
async def handler(msg: str) -> None:
...

Constructing the StreamSub prints the warning. Running the app shows that the flag is honoured anyway: XPENDING my-stream my-group stays empty after messages are consumed.

Expected behavior

No warning. no_ack=True with a consumer group is a supported, documented mode ("equivalent to acknowledging the message when it is read", see docs/docs/en/redis/streams/groups.md).

Observed behavior

The warning is emitted in faststream/redis/schemas/stream_sub.py (the elif no_ack: branch under if group and consumer:), while the rest of the code base treats the flag as live:

faststream/redis/subscriber/usecases/stream_subscriber.py passes noack=stream.no_ack to XREADGROUP;
faststream/redis/subscriber/config.py switches the subscriber to AckPolicy.MANUAL when no_ack is set;
faststream/redis/testing.py skips the PEL for no_ack subscribers;
#3049 added a SetupError for claim_min_idle_time + no_ack, which only makes sense if no_ack works with groups.

History

• The warning was introduced in 0.3.5 (#1048), when no_ack was indeed not forwarded to XREADGROUP.
#2309 kept the warning and reset no_ack = False right after it, so the flag really was ignored for a while.
• 0.6.0 (#1779) dropped that reset, and noack=stream.no_ack has been sent to Redis since, but the warning stayed.

The sibling warning, "no_ack is not supported by consumer group with last_id other than >", is a separate case and should be checked on its own: Redis documents NOACK for XREADGROUP regardless of the id, so it may be stale too.

Suggested fix

Remove the elif no_ack: warning branch in StreamSub.__init__ and add a test asserting that StreamSub("s", group="g", consumer="c", no_ack=True) raises no warning and that the subscriber calls xreadgroup with noack=True. Keep the claim_min_idle_time + no_ack SetupError as is.

Environment

Reproduced on main (commit 0307f09).


#bug #good_first_issue #redis #faststream #ag2ai
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 `OpenAPI.json_schema_dialect` is never set (#1486)


We must add json_schema_dialect to OpenAPIConfig, because currently there's no way to set json_schema_dialect attribute.


#feature #good_first_issue #help_wanted #openapi #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Make `ConfigMerge` class customizable (#1487)


Currently, there's no way to customize the ConfigMerger class here:

django-modern-rest/dmr/openapi/core/context.py

Lines 61 to 69 in f822e35

This goes against our design, where we provide all classes as ClassVar[type[ConfigMerger]] objects.

It should be:

class OpenAPIContext:
config_merger_cls: ClassVar[type[ConfigMerger]] = ConfigMerger

# then:
# self.config_merger = self.config_merger_cls(self)


#feature #good_first_issue #help_wanted #openapi #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Make sure that `SecuritySchemeRegistry` can't re-register existing auth schemes (#1488)


Currently SecuritySchemeRegistry does not protect from registerting two auth schemes with the same name:

django-modern-rest/dmr/openapi/core/registry.py

Lines 127 to 142 in f822e35

This needs to be fixed. It must raise an error, no error must pass silently.


#bug #good_first_issue #help_wanted #openapi #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Support custom `format` options, add more pre-defined formats (#1489)


Currently we only allow format strings that are present in OpenAPIFormat enum. Which is not correct, because format is an open str type. Users can create custom formats. See the spec: https://spec.openapis.org/oas/v3.2.0.html#data-type-format

Also, add all formats from https://spec.openapis.org/registry/format/ to OpenAPIFormat just for the usability.

Also, please make sure that these types work correctly in tests/test_unit/test_plugins/test_pydantic/test_pydantic_snapshots.py:

pydantic.NameEmail
pydantic.Base64Str
pydantic.Base64Bytes
• Custom format with json_schema_extra={'custom-format': ...} for regular str field in pydantic

#bug #good_first_issue #help_wanted #openapi #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Fix missing `Schema` attributes in `load_schema` (#1490)


In my personal opinion load_schema is the ugliest part of the framework. New issues proposing better design of this feature is MASSIVELY welcome. But, before that we need to fix the existing problem.

Here's how a schema is loaded right now:

django-modern-rest/dmr/openapi/mappers/schema_loader.py

Lines 67 to 127 in f822e35

We never set: anchor, comment, ref, and schema_uri. Maybe others?

But, $ref is the most complex one here. Please, ignore it for now, we must fix our Reference | Schema handling in all places at once 😢


#bug #good_first_issue #help_wanted #openapi #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Fix `Reference | Schema` conflict in our OpenAPI definitions (#1491)


Currently we mix up two different things in the code: Schema with $ref set and a Reference object with $ref set.

For example, this code:

import pydantic
from pydantic.json_schema import GenerateJsonSchema
from dmr.openapi.mappers.schema_loader import load_schema


class WithDialect(GenerateJsonSchema):
def generate(self, schema, mode='validation'):
json_schema = super().generate(schema, mode=mode)
json_schema['$schema'] = self.schema_dialect
return json_schema


class Address(pydantic.BaseModel):
model_config = pydantic.ConfigDict(
json_schema_extra={'$anchor': 'address', '$comment': 'Postal address'},
)
city: str


class User(pydantic.BaseModel):
model_config = pydantic.ConfigDict(
json_schema_extra={'$comment': 'Internal note for schema readers'},
)
name: str = pydantic.Field(json_schema_extra={'$comment': 'Display name'})
address: Address = pydantic.Field(
default=Address(city='Moscow'),
description='Where the user lives',
)


raw = pydantic.TypeAdapter(User).json_schema(
ref_template='#/components/schemas/{model}',
schema_generator=WithDialect,
)
defs = raw.pop('$defs')

Produces this schema:

{
"$comment": "Internal note for schema readers",
"$schema": "https://json-schema.org/draft/2020-12/schema",
"properties": {
"name": {"$comment": "Display name", "title": "Name", "type": "string"},
"address": {
"$ref": "#/components/schemas/Address",
"default": {"city": "Moscow"},
"description": "Where the user lives"
}
},
"required": ["name"], "title": "User", "type": "object"
}

But, we would load it as Reference object in load_schema and we will loose default and description. Which are Schema object attributes.

So, what we need to do?

1. Analyze all places where Reference can't be even used based on https://spec.openapis.org/oas/v3.2.0.html We need to grep this page with | Reference Object and check that we don't have more places that can hold Reference objects
2. We must distinguish $ref in potential Schema objects and Reference objects
3. We must change how our maybe_resolve_reference works to also resolve $ref in Schema objects, where we need it. Maybe we create a new type ResolvedSchema and use it in places where we expect flat schemas, so we won't forget to call maybe_resolve_reference

#bug #good_first_issue #help_wanted #openapi #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Change how we work with `FileMetadata` validation (#1493)


Currently, this code is allowed:

from typing import Literal

import pydantic

from dmr import Body, Controller, FileMetadata
from dmr.plugins.pydantic import PydanticSerializer
from dmr.parsers import JsonParser, MultiPartParser


class Text(pydantic.BaseModel):
content_type: Literal['text/plain']


class Files(pydantic.BaseModel):
text: Text


class MyController(Controller[PydanticSerializer]):
parsers = [JsonParser(), MultiPartParser()]

def post(
self,
parsed_body: Body[dict[str, str]],
parsed_file_metadata: FileMetadata[Files],
) -> str:
return 'done'

But, how can we ever satisfy?

JsonParser()
parsed_file_metadata: FileMetadata[Files]

It is not optional. Every request here will fail.
What we can do? Change

django-modern-rest/dmr/components.py

Lines 866 to 889 in f822e35

from any to all.


#bug #good_first_issue #help_wanted #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Test and document that default values for components are supported (#1494)


Currently we don't ever test or document that a code like this:

from dmr import Body, Controller
from dmr.plugins.pydantic import PydanticSerializer


class MyController(Controller[PydanticSerializer]):
def post(
self,
parsed_body: Body[dict[str, str] | None] = None,
) -> str:
print(parsed_body)
return 'done'

is even supported.

What is important here?

1. We must call the endpoint function as-is, without any work from our part. Defaults must be real python defaults. And work the same way
2. But, if there are default for Body, we must issue required: false schema for the whole body

Snapshot test is required for this change.


#good_first_issue #help_wanted #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 It is possible to produce an empty `description` for `ResponseBody` (#1495)


django-modern-rest/dmr/openapi/generators/component_parsers.py

Lines 193 to 199 in f822e35

It misses or None part. Because we only skip None values from dumping.


#bug #good_first_issue #help_wanted #openapi #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Test that nested URL patterns produce correct parameters (#1496)


We need to test a case like:

Router('tenants/<int:tenant_id>/', [path('users/<int:pk>/', MyController.as_view())])

It might contain a bug: we should get two path parameters, not one.


#good_first_issue #help_wanted #openapi #opensource_september #django_modern_rest
sent via relator
🚀 New issue to wemake-services/django-modern-rest by @sobolevn
📝 Test explicit `OpenAPIConfig.security` definition with `auth=None` per-endpoint (#1497)


Looks like we have a potential bug in how we define security in OpenAPI.
Here we always return None:

django-modern-rest/dmr/openapi/generators/security_scheme.py

Lines 24 to 37 in f822e35

But, if per-document security is set, we need to return [] as the value. Otherwise, None won't be dumped by our None-ignoring dump_schema rules. And security rules per this endpoint will be inherited from the document.

Which is wrong.

This would require a test and a fix. Maybe we should look at current config's security to define the return type?


#bug #good_first_issue #help_wanted #openapi #opensource_september #django_modern_rest
sent via relator