-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add examples #18
Add examples #18
Conversation
Reviewer's Guide by SourceryThis pull request adds three example files demonstrating the usage of the Class diagram for the new example scriptsclassDiagram
class jsonplaceholder_api_example {
+requests.get(url)
+trace(post_data, 'title')
+trace(comments_data[0], 'email')
}
class openweathermap_api_example {
+requests.get(url)
+trace(data, 'temp')
+trace(data, 'description')
}
class github_api_example {
+requests.get(url)
+trace(data, 'stargazers_count')
+trace(data, 'login')
}
class trace_dkey {
+trace(data, key)
}
jsonplaceholder_api_example --> trace_dkey
openweathermap_api_example --> trace_dkey
github_api_example --> trace_dkey
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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 @ssghait007 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider removing the hardcoded API key in the OpenWeatherMap example and use a more secure method for handling API keys, such as a configuration file.
- The examples could benefit from some refactoring to reduce code duplication. Consider creating a common function for making API requests and processing responses.
- To enhance the educational value of these examples, consider adding more detailed comments explaining the trace-dkey functionality and implement error handling for API requests.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 4 issues found
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
def jsonplaceholder_api_example(): | ||
# Get a sample post | ||
url = "https://jsonplaceholder.typicode.com/posts/1" | ||
response = requests.get(url) |
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: Consider adding error handling for API responses
It's important to handle potential exceptions when making API calls. You could use a try-except block to catch and handle requests.exceptions.RequestException.
try:
response = requests.get(url)
response.raise_for_status()
except requests.exceptions.RequestException as e:
print(f"An error occurred: {e}")
return
Thanks @ssghait007 , resolve these comments, I have disabled Sourcery, feels like unnecessary noise |
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
fixes #11
Summary by Sourcery
Introduce example scripts for JSONPlaceholder, OpenWeatherMap, and GitHub APIs to demonstrate data fetching and path tracing using the trace_dkey module.
New Features: