Skip to content

Database: change db arch#67

Open
Spotika wants to merge 10 commits into
masterfrom
fix/db-methods
Open

Database: change db arch#67
Spotika wants to merge 10 commits into
masterfrom
fix/db-methods

Conversation

@Spotika

@Spotika Spotika commented Sep 25, 2024

Copy link
Copy Markdown
Member

Все методы с бд в одном файле

Удалены коллекции, которые не используются

Участники организации хранятся в отдельной коллекции с compound index

/api/organizations/get_members

Comment thread src/db/methods/methods.py Outdated
Comment thread src/routers/auth.py Outdated
@difhel

difhel commented Sep 26, 2024

Copy link
Copy Markdown
Member

Кажется, должно быть по другому: в отдельных файлах определяешь методы для работы с базой данных, а в сабмодуле db/methods/__init__.py что-то подобное:

from .users import *
from .organisations import *__all__ = [
    "get_user",
    "get_org",
    ...,
]

@difhel difhel assigned Spotika and unassigned difhel Sep 26, 2024
@Spotika

Spotika commented Sep 26, 2024

Copy link
Copy Markdown
Member Author

Кажется, должно быть по другому: в отдельных файлах определяешь методы для работы с базой данных, а в сабмодуле db/methods/init.py что-то подобное:

from .users import *
from .organisations import *__all__ = [
    "get_user",
    "get_org",
    ...,
]

Мы не будем использовать all в таком контексте. В качестве инструмента, который запрещает использовать экспортируемые символы у нас pylint

Что касаемо разных файлов, то можно работать в рамках одного файла со всеми методами
Да, экспортировать все файлы идея хорошая, но нужно именовать методы без ссылок на тот файл где они находятся

@Spotika Spotika assigned difhel and unassigned Spotika Sep 26, 2024
@Spotika Spotika requested a review from Copilot November 19, 2024 18:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot reviewed 11 out of 24 changed files in this pull request and generated no suggestions.

Files not reviewed (13)
  • src/db/methods/domains.py: Evaluated as low risk
  • src/db/types/types.py: Evaluated as low risk
  • src/routers/users.py: Evaluated as low risk
  • src/routers/auth.py: Evaluated as low risk
  • src/routers/test.py: Evaluated as low risk
  • src/utils/auth/auth.py: Evaluated as low risk
  • src/typings/requests.py: Evaluated as low risk
  • tests/src/helpers/api.ts: Evaluated as low risk
  • src/config/config_model.py: Evaluated as low risk
  • src/db/methods/organizations.py: Evaluated as low risk
  • src/db/methods/collections/collections.py: Evaluated as low risk
  • src/typings/responses.py: Evaluated as low risk
  • src/db/methods/collections/init.py: Evaluated as low risk
Comments skipped due to low confidence (2)

src/routers/organizations.py:15

  • The error message for ErrorCodes.NOT_FOUND is missing. It should provide a clear message indicating that the organization was not found.
raise ErrorResponse(code=ErrorCodes.NOT_FOUND)

src/routers/organizations.py:24

  • The new behavior in the get_members method should be covered by tests to ensure it works as expected.
return RS.organizations.get_members(members=methods.get_members_of_organization(request.id))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants