Creates repository sql database#25 #35
Conversation
EliuX
force-pushed
the
feature/Create-repository-sql-database#25
branch
from
a2898a8 to
cb6104b
Compare
tests/resources.py
Outdated
| from time_tracker_api.sql_repository import db, SQLAuditedModel | ||
|
|
||
|
|
||
| class PersonSQLModel(db.Model, SQLAuditedModel): |
There was a problem hiding this comment.
What's the reason to have SQL as part of the class name?
There was a problem hiding this comment.
Because this is the SQLAlchemy model, not a generic one
There was a problem hiding this comment.
is it extremelly necessat to include SQL as part of the name?
There was a problem hiding this comment.
Yes, because this model is specific to the SQL implementation. As well as I have the SQL prefix in the repository and other resources related to this particular technology, which has its own characteristics: specific library, handling of commits, rollbacks, the concept of tables, columns and so forth, which is not the same as, for instance, an In-memory implementation, a Mongo Implementation, etc.
There was a problem hiding this comment.
Just by looking it has the SQL prefix you know that this has SQL-specific logic.
There was a problem hiding this comment.
We are not going to add support for Mongo, in-memory or so. We should not pollute code with implementation details.
EliuX
force-pushed
the
feature/Create-repository-sql-database#25
branch
4 times, most recently
from
604e8dd to
abf19f8
Compare
EliuX
force-pushed
the
feature/Create-repository-sql-database#25
branch
from
abf19f8 to
bafb867
Compare
tests/sql_repository_test.py
Outdated
|
|
||
|
|
||
| def test_create(sql_repository): | ||
| """Should create a new Entry""" |
There was a problem hiding this comment.
why not naming the function/method name as
test_creates_a_new_entry
Then you can get rid of comments
There was a problem hiding this comment.
This function just tests the function create of the repository, this is a simple and unique name. As I explained before the comment in the function is providing a verbose description of the test.
tests/sql_repository_test.py
Outdated
|
|
||
|
|
||
| def test_update(sql_repository): | ||
| """Updates an existing element""" |
There was a problem hiding this comment.
no need to have comments, please remove them and make test name consistent
There was a problem hiding this comment.
In Java I would not have put comments, but this is Python. It is common here put comments even in modules. This is a test that tests the function called update. This is very simple.
|
|
||
| return ProjectSQLDao() | ||
| else: | ||
| """ |
There was a problem hiding this comment.
I think we should get rid of all the in-memory implementation.
- We will need to implement this in-memory code for all DAOs
- We are not going to need this in-memory solution
- A Dao should has more implementations if we really need them. For now, just one sql implementation is good enough
There was a problem hiding this comment.
This in-memory code will not be used, this is just basically for documentation purposes. I wanted the other people who work in the project to see how you can create other db mechanisms. And now that the Azure database serverless implementation is showing some issues, it is nice to be aware of how this work.
EliuX
force-pushed
the
feature/Create-repository-sql-database#25
branch
from
a6e994c to
336eb75
Compare
EliuX
force-pushed
the
feature/Create-repository-sql-database#25
branch
from
d75da71 to
2ed13c9
Compare
This PR creates the base for using the SQL Repository in endpoint queries, so we are ready to persist data.