Skip to content

Review User Story #19#54

Open
ghost wants to merge 4 commits into
mainfrom
feature/19-historical-data-storing-viewing
Open

Review User Story #19#54
ghost wants to merge 4 commits into
mainfrom
feature/19-historical-data-storing-viewing

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Nov 10, 2023

Hello, take time to review what i have done so far. I appreciate your feedback.

@ghost ghost requested review from ArmandMeppa and stephane-segning November 10, 2023 11:31
Copy link
Copy Markdown
Contributor

@stephane-segning stephane-segning left a comment

Choose a reason for hiding this comment

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

Very good work 😎, as always @sinke237. But you missed a point. Look at this CPU Service https://github.com/ADORSYS-GIS/monitor-mind/blob/main/services/cpu_service.py#L6 where I cheated and used a Python dictionary to save data to, in a key-value form. What I'm expecting from you, is to make this saving better, by, as you already started, using sqlite there 👌🏿

Comment thread app.py Outdated
Comment thread app.py Outdated

return render_template('historic_data.html', data=data)

def collect_metrics():
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.

Very good! Now you can put this into each service, instead of in the main entry point of this app 😊

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

put it into each service, I'm sorry I am sure I understand what you mean.

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.

You could see that each resource had a single responsible in "services" folder. The idea is to separate each

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Wait, so I am supposed to have 11 .py files inside the services folder??

Comment thread services/__pycache__/cpu_service.cpython-310.pyc Outdated
Comment thread services/__pycache__/memory_service.cpython-310.pyc Outdated
Comment thread services/__pycache__/time_service.cpython-310.pyc Outdated
Comment thread templates/historic_data.html
ghost pushed a commit that referenced this pull request Nov 10, 2023
ghost pushed a commit that referenced this pull request Nov 10, 2023
@ghost ghost force-pushed the feature/19-historical-data-storing-viewing branch from e9c493f to dbc9889 Compare November 11, 2023 09:30
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.

2 participants