Skip to content

Refactor magstats to fast api#173

Open
mmolina2018 wants to merge 21 commits intomainfrom
refactor-magstats-to-FastAPI
Open

Refactor magstats to fast api#173
mmolina2018 wants to merge 21 commits intomainfrom
refactor-magstats-to-FastAPI

Conversation

@mmolina2018
Copy link
Copy Markdown

No description provided.

@mmolina2018 mmolina2018 linked an issue Sep 25, 2023 that may be closed by this pull request
Comment thread magstats/Dockerfile
Comment on lines +25 to +28
COPY ./entrypoint.sh /app/
COPY ./core /app/core
COPY ./api /app/api
COPY ./database /app/database
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

El path acá no está correcto

Comment thread magstats/tests/test_service.py Outdated
Comment on lines +31 to +43
class TestGetMagstatsSQL(unittest.TestCase):
def test_db_exception(self):
def mock_session_factory_with_exception():
raise Exception("Test exception")

with pytest.raises(HTTPException) as exc:
get_magstats(
session_factory=mock_session_factory_with_exception,
oid="oid1",
handle_error=handle_error,
)

assert exc.value.status_code == 500
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Por qué poner esto en una clase y los demás tests no ?

Copy link
Copy Markdown
Contributor

@dirodriguezm dirodriguezm left a comment

Choose a reason for hiding this comment

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

También veo que falta la parte de CD. Sin eso no tiene mucho sentido aprobar el PR.

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.

Refactorizar Magstats multistream

2 participants