agronholm / agronholm/sqlacodegen

Using types instead of strings in return values of methods in generators

オープン
#328 コメント 0 件 リアクション 3 件 担当者 0 名 GitHub で見る
enhancement
主要言語
Python
スター
2.4k
フォーク
284
平均マージ
2日 1時間
マージ済み PR(30日)
1

説明

### Things to check first

- [X] I have searched the existing issues and didn't find my feature already requested there

### Feature description

## Usecase
Due to historical reasons I have to deal with a codebase where we are converting Uuid's to string when we fetch them from database, as a result we need to look at the custom generator logic in particular the following functions
* `render_column_type`
* `render_column_attribute`

## The current custom generator

Right now both these functions return `str` which is hard to make changes too. I was wondering if you would be open to modifying these interfaces to return some types here, so these types are easy to modify.

I will take the example of modifying these for the aforementioned use case. To satisfy the condition I have done the following

```python
def render_column_type(self, coltype: object) -> str:
from_lib = super().render_column_type(coltype)
if from_lib == "Uuid" and isinstance(coltype, Uuid):
return render_callable(coltype.__class__.__name__, kwargs={"as_uuid": False})
return from_lib

def render_column_attribute(self, column_attr: ColumnAttribute) -> str:
column_python_type = "str"
column = column_attr.column
rendered_column = self.render_column(column, column_attr.name != column.name)

if column.nullable:
self.add_literal_import("typing", "Optional")
column_python_type = f"Optional[{column_python_type}]"

if isinstance(column_attr.column.type, Uuid):
return f"{column_attr.name}: Mapped[{column_python_type}] = {rendered_column}"

return super().render_column_attribute(column_attr)
```

As you can see this is a lot of code for something extremely trivial.

## Proposed feature (or rather modification)

Let's assume we had types like the following
```python
class RenderedColumnType:
coltype: object
args: list[str]
kwargs: dict[str, str]

class RenderedColumnAttribute:
column_attr: ColumnAttribute
column_python_base_type: str
nullable: bool
rendered_column: RenderedColumn
```

And the methods on the generators had the following interface

```python
def render_column_type(self, coltype: object) -> RenderedColumnType:
def render_column_attribute(self, column_attr: ColumnAttribute) -> RenderdColumnAttribute:
```

The the code that a consumer of this API would have to write would have been something like

## The Custom Generator with these proposed changes
```python
def render_column_type(self, coltype: object) -> RenderedColumnType:
from_lib = super().render_column_type(coltype)
if isinstance(coltype, Uuid):
from_lib.kwargs["as_uuid"] = False
return from_lib

def render_column_attribute(self, column_attr: ColumnAttribute) -> RenderedColumnAttribute:
from_lib = super().render_column_attribute(column_attr)
if isinstance(column_attr.column.type, Uuid):
from_lib.column_python_base_type = "str"
return from_lib
```

Which, imo, offers a far simpler interface for the consumer of this API to work with.

I have to admit that I have only thought about this from the use case that I encountered, and have not considered the overall impact on the generators. For that I can definitely use your insights, and lastly if this is something you would be interested in pursuing let me know, I can try to cook a PR to test out how this will look like.

Last but certainly not least, thanks for this awesome project it has been super useful.

### Use case

Due to historical reasons I have to deal with a codebase where we are converting Uuid's to string when we fetch them from database, as a result we need to look at the custom generator logic in particular the following functions
* `render_column_type`
* `render_column_attribute`

Most of the details about this are already covered in the feature description section.

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。