-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adicionado rotinas de monitoramento e testes de unidade #1
base: base-sha/a331f84b2c64dd2e5be3359b5f196a8145b67ab2
Are you sure you want to change the base?
Conversation
… e ajustes nas dependencias
This is a benchmark review for experiment This pull request was cloned from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @brendanator - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟡 Docstrings: 1 issue found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
'console': { | ||
'level': 'WARNING', | ||
'class': 'logging.StreamHandler', | ||
'stream': os.sys.stdout, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code_refinement): Consider configuring log file output for better log management.
Directing logs to stdout is convenient for development but consider also configuring a file handler for more persistent log management, aiding in debugging past issues.
'stream': os.sys.stdout, | |
'stream': os.sys.stdout, | |
'handlers': ['console', 'fileHandler'], | |
}, | |
}, | |
'handlers': { | |
'fileHandler': { | |
'level': 'WARNING', | |
'class': 'logging.FileHandler', | |
'filename': 'app_logs.log', | |
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment helpful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the comment type correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the comment area correct?
@@ -18,3 +18,14 @@ def test_currency_data_str(self): | |||
|
|||
expected_object_name = f"{test_pair} - {test_timestamp.strftime('%d/%m/%Y %H:%M:%S')}" | |||
self.assertEqual(str(currency_data), expected_object_name) | |||
|
|||
|
|||
class ExecutionLogModelTest(TestCase): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (testing): Consider adding a failure case test for ExecutionLog.
It's great to see tests for the successful creation of ExecutionLog entries. However, adding a test case to simulate and verify the behavior of ExecutionLog under failure conditions, such as when an error status is logged, would enhance the test coverage.
class ExecutionLogModelTest(TestCase): | |
def test_execution_log_failure_condition(self): | |
"""Test the behavior of ExecutionLog under failure conditions.""" | |
time_now = timezone.now() | |
log = ExecutionLog(status=500, log="Error during execution", created_at=time_now) | |
expected_str = f"ExecutionLog {time_now} - Status 500" | |
self.assertEqual(str(log), expected_str) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment helpful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the comment type correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the comment area correct?
|
||
class Command(BaseCommand): | ||
help = "Carrega dados históricos de criptomoedas e calcula médias móveis simples MMS" | ||
|
||
def handle(self, *args, **kwargs): | ||
def handle(self, *args, **kwargs): # sourcery skip: extract-method |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (docstrings): Please update the docstring for function:
handle
Reason for update: The function's behavior has been extended to include logging of the data loading process and errors.
Suggested new docstring:
Carrega dados históricos de criptomoedas, calcula médias móveis simples (MMS) e registra o processo e erros.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this comment helpful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the comment type correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the comment area correct?
Adicionado rotinas de monitoramento e testes de unidade