Future-Energy-Associates / granular_certificate_registry

An open-source platform to demonstrate the capabilities of a Granular Certificate registry that conforms to the EnergyTag Standards and API specification.
Apache License 2.0
0 stars 0 forks source link

29 Directory Refactor #36

Closed CAGalbraith closed 1 month ago

CAGalbraith commented 1 month ago

Description

Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

Checklist:

CAGalbraith commented 1 month ago

Why would mypy pass locally but fail on the CI?

ClemAtt commented 1 month ago

Try using ‘make pre-commit’ if not already. On 9 Sep 2024 at 19:00 +0100, Connor Galbraith @.***>, wrote:

Why would mypy pass locally but fail on the CI? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

CAGalbraith commented 1 month ago

Still passing locally, hmm

image

ClemAtt commented 1 month ago

Still passing locally, hmm

image

Looks like dependency issues. Try adding the stubs for mypy libraries and making sure the poetry toml and .lock are up to date:

gc_registry/client/authentication.py:81: error: Module has no attribute "UTC"  [attr-defined]
gc_registry/client/authentication.py:83: error: Module has no attribute "UTC"  [attr-defined]
gc_registry/client/authentication.py:117: error: Module has no attribute "UTC"  [attr-defined]
gc_registry/main.py:6: error: Library stubs not installed for "markdown"  [import-untyped]
gc_registry/main.py:6: note: Hint: "python3 -m pip install types-Markdown"
gc_registry/main.py:6: note: (or run "mypy --install-types" to install all missing stub packages)
gc_registry/main.py:6: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 4 errors in 2 files (checked 41 source files)
make: *** [Makefile:[21](https://github.com/Future-Energy-Associates/granular_certificate_registry/actions/runs/10778562498/job/29890169421?pr=36#step:6:22): typecheck] Error 1
ClemAtt commented 1 month ago

For schemas vs models the distinction we are using in Tariff Tribe is that:

Is this what you were thinking for this? Based on the above definition schemas inherit from BaseModel and models inherit from SQLModel. It seems using BaseModel has less overhead than using SQLModel, so we may want to refactor depending on how we are defining the above. Let's create another ticket for this as required.

Where you following a particular DDD approach for top level structure using client, CRUD, models, schemas? Just so I can understand the logic of the approach. I would have expected the CRUD directory to include methods for interacting with the DB rather than API routes.

CAGalbraith commented 1 month ago

For schemas vs models the distinction we are using in Tariff Tribe is that:

Not sure why I didn't look through the layout of TT beforehand but I like that approach a lot more - it's still a little rough around the edges and I think there are some more changes that can be made between models and schemas, as well as adding enums in the core directory, but otherwise pretty happy with this approach.