Skip to content
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

feat: Remove sqlmodel #165

Merged
merged 3 commits into from
Oct 21, 2022
Merged

feat: Remove sqlmodel #165

merged 3 commits into from
Oct 21, 2022

Conversation

thulio
Copy link
Collaborator

@thulio thulio commented Aug 7, 2022

Esse PR faz amora depender apenas da lib sqlachemy, a qual tem suporte nativo para type-hinting

@thulio thulio self-assigned this Aug 7, 2022
@thulio thulio force-pushed the feat/remove-sqlmodel branch 24 times, most recently from 93d8b02 to 94dcd24 Compare August 7, 2022 18:10
@diogommartins diogommartins self-requested a review August 8, 2022 14:47
@diogommartins
Copy link
Contributor

Foda, @thulio !!! Ótima iniciativa 😻

@thulio
Copy link
Collaborator Author

thulio commented Aug 9, 2022

Vou quebrar esse PR em partes menores/não relacionadas

@thulio thulio mentioned this pull request Aug 9, 2022
@thulio thulio force-pushed the feat/remove-sqlmodel branch 2 times, most recently from fc8b256 to 7d51243 Compare October 20, 2022 18:21
@thulio thulio changed the title wip: Remove sqlmodel feat: Remove sqlmodel Oct 20, 2022
@thulio thulio marked this pull request as ready for review October 20, 2022 18:23
@thulio thulio added enhancement New feature or request help wanted Extra attention is needed dependencies Pull requests that update a dependency file labels Oct 20, 2022
@thulio thulio force-pushed the feat/remove-sqlmodel branch 2 times, most recently from 2bbf8a9 to 640382b Compare October 20, 2022 18:41
@diogommartins
Copy link
Contributor

image
Pode remover essa referência do README também, por favor?

for dependency in getattr(task.model, "__depends_on__", []):
dag.add_edge(dependency.unique_name(), task.model.unique_name())
for dependency in task.model.dependencies():
dag.add_edge(dependency.fullname, task.model.unique_name())
Copy link
Contributor

Choose a reason for hiding this comment

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

Por que unique_name() não é mais usado?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Porque dependency é do tipo Table e não AmoraModel.

Para AmoraModel, unique_name() == __table__.fullname


@classmethod
def dependencies(cls) -> Models:
def dependencies(cls) -> List[Table]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Algum motivo pra essa mudança ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

O motivo tava no todo comentado:

        # todo: Remover necessidade de __depends_on__ inspecionando a query e chegando ao modelo de origem
        # tables: List[Table] = source.froms

amora/models.py Outdated Show resolved Hide resolved
@diogommartins
Copy link
Contributor

🚀🚀🚀🚀🚀🚀

@thulio thulio merged commit c193abb into main Oct 21, 2022
@thulio thulio deleted the feat/remove-sqlmodel branch October 21, 2022 14:35
@diogommartins diogommartins mentioned this pull request Oct 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants