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(results): Basic results implementation #158

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

caviri
Copy link
Contributor

@caviri caviri commented Jun 24, 2024

This is the initial implementation for results.

One digital twin can have one first result associated. This will be generated automatically after the digital twin generation.

Copy link
Contributor

@sabinem sabinem left a comment

Choose a reason for hiding this comment

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

@caviri I accidentally started to review this PR as well, but then noticed that you haven't requested a review yet, since I think this PR iw not ready for review yet, correct? Anyway maybe my comment will be helpful to you in moving forward.

),
):
try:
if dt_name is None and dt_id is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: I think overall the PR iff fine for me. but I would suggest to put the logic into the db function rather then in the cli command. There you can raise an exception if something goes wrong such as if the S3 connection can not be established. This way you will also better know what happened, whereas if you have the logic in the cli the command will just stop at a certain point in the try except block.

I think it is just best practice to not have the logic in the cli command but rather in function that you call from the cli. That way the logic can also be reused later by the dashboard for example.

@sabinem sabinem changed the title feat(results): Basic results implementation relase 0.4.0 feat(results): Basic results implementation Jul 2, 2024
@sabinem sabinem changed the title relase 0.4.0 feat(results): Basic results implementation feat(results): Basic results implementation Jul 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants