-
-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
Owner
...g Bootstrap. It also makes the application production-ready by adding a requirements.txt file, a Procfile for Gunicorn, and configuring the app to use environment variables.
The main changes are:
- Replaced the old stylesheet with Bootstrap 5.
- Updated all major templates to use Bootstrap components for a modern and responsive UI.
- Added a
requirements.txtfile with all the necessary dependencies. - Added a
Procfileto run the application with Gunicorn. - Updated the configuration to use
python-dotenvto load environment variables. - Fixed several bugs related to missing dependencies and application startup hangs.
Summary by Sourcery
Revamp the application's UI with Bootstrap 5, make it production-ready by adding dependency and deployment configurations, and enhance API routes with error handling and timeouts.
New Features:
- Introduce Bootstrap 5 for modern, responsive UI across all main templates
- Add requirements.txt to pin project dependencies
- Include a Procfile to run the application with Gunicorn
- Configure python-dotenv to load environment variables in application config
- Implement request timeouts and exception handling for UNESCO and WHO API endpoints
Bug Fixes:
- Fix missing dependencies and prevent startup hangs by specifying dependencies and adding request timeouts
...g Bootstrap. It also makes the application production-ready by adding a `requirements.txt` file, a `Procfile` for Gunicorn, and configuring the app to use environment variables.
The main changes are:
- Replaced the old stylesheet with Bootstrap 5.
- Updated all major templates to use Bootstrap components for a modern and responsive UI.
- Added a `requirements.txt` file with all the necessary dependencies.
- Added a `Procfile` to run the application with Gunicorn.
- Updated the configuration to use `python-dotenv` to load environment variables.
- Fixed several bugs related to missing dependencies and application startup hangs.
The main changes are:
- Replaced the old stylesheet with Bootstrap 5.
- Updated all major templates to use Bootstrap components for a modern and responsive UI.
- Added a `requirements.txt` file with all the necessary dependencies.
- Added a `Procfile` to run the application with Gunicorn.
- Updated the configuration to use `python-dotenv` to load environment variables.
- Fixed several bugs related to missing dependencies and application startup hangs.
Reviewer's GuideThis PR overhauls the application's UI/UX by integrating Bootstrap 5 and refactoring all major templates into responsive layouts, configures production deployment with Gunicorn and dotenv support, and strengthens reliability through robust external API error handling. Flow diagram for improved external API error handling
flowchart TD
A[User requests WHO/UNESCO data] --> B[App sends API request] B -->|Success| C[Parse and display data] B -->|Error/Timeout| D[Show error message to user] File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Vulnerable Libraries (3)
More info on how to fix Vulnerable Libraries in Python. Go to the dashboard for detailed results. Happy? Share your feedback with us. |
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 @GYFX35 - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Running with debug=True in production is a security risk. (link)
General comments:
- Pin dependency versions in requirements.txt to ensure reproducible builds and avoid unexpected upgrades.
- Remove debug=True from run.py in production mode and control debug configuration via environment variables.
- Add SRI integrity and crossorigin attributes to your CDN Bootstrap and jQuery script tags to improve security.
Prompt for AI Agents
{{ message }}
+ {% endfor %}
+ {% endif %}
{{ message }}
{% endfor %}
{% endif %}
=======
{% set bootstrap_alert_map = {
'error': 'danger',
'message': 'info',
'info': 'info',
'success': 'success',
'warning': 'warning',
'danger': 'danger'
} %}
{% with messages = get_flashed_messages(with_categories=true) %}
{% if messages %}
{% for category, message in messages %}
{{ message }}
{% endfor %}
{% endif %}
>>>>>>> REPLACE
## Overall Comments
- Pin dependency versions in requirements.txt to ensure reproducible builds and avoid unexpected upgrades.
- Remove debug=True from run.py in production mode and control debug configuration via environment variables.
- Add SRI integrity and crossorigin attributes to your CDN Bootstrap and jQuery script tags to improve security.
## Individual Comments
### Comment 1
<location> `app/routes.py:208` location>
- url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json"
- response = requests.get(url)
- data[name] = response.json()['dataSets'][0]['series']
+ try:
+ for code, name in indicators.items():
+ url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json"
+ response = requests.get(url, timeout=10)
Consider more granular error handling for UNESCO API requests.
Wrapping each API request in its own try/except block would allow the function to return available data even if some requests fail.
<<<<<<< SEARCH
try:
for code, name in indicators.items():
url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json"
response = requests.get(url, timeout=10)
response.raise_for_status()
data[name] = response.json()['dataSets'][0]['series']
except requests.exceptions.RequestException as e:
flash(f"Could not retrieve data from UNESCO API: {e}")
data = {}
=======
for code, name in indicators.items():
url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json"
try:
response = requests.get(url, timeout=10)
response.raise_for_status()
data[name] = response.json()['dataSets'][0]['series']
except requests.exceptions.RequestException as e:
flash(f"Could not retrieve data for '{name}' from UNESCO API: {e}", "warning")
>>>>>>> REPLACE
### Comment 2
<location> `templates/base.html:77` location>
+ {% with messages = get_flashed_messages(with_categories=true) %}
+ {% if messages %}
+ {% for category, message in messages %}
+
{{ message }}
+ {% endfor %}
+ {% endif %}
Flashed message categories may not map directly to Bootstrap alert classes.
If your flash categories differ from Bootstrap's expected values, map them accordingly to ensure correct alert styling.
<<<<<<< SEARCH
{% with messages = get_flashed_messages(with_categories=true) %}
{% if messages %}
{% for category, message in messages %}
<div class="alert alert-{{ category }}">{{ message }}div>
{% endfor %}
{% endif %}
=======
{% set bootstrap_alert_map = {
'error': 'danger',
'message': 'info',
'info': 'info',
'success': 'success',
'warning': 'warning',
'danger': 'danger'
} %}
{% with messages = get_flashed_messages(with_categories=true) %}
{% if messages %}
{% for category, message in messages %}
{% endfor %}
{% endif %}
>>>>>>> REPLACE
### Comment 3
<location> `run.py:4` location>
if __name__ == '__main__':
- app.run(debug=True)
+ app.run(host='0.0.0.0', port=8080, debug=True)
Running with debug=True in production is a security risk.
Use environment variables or configuration files to manage the debug setting and ensure it is disabled in production.
Help me be more useful! Please click or on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+208
to
+216
| try: | ||
| for code, name in indicators.items(): | ||
| url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json" | ||
| response = requests.get(url, timeout=10) | ||
| response.raise_for_status() | ||
| data[name] = response.json()['dataSets'][0]['series'] | ||
| except requests.exceptions.RequestException as e: | ||
| flash(f"Could not retrieve data from UNESCO API: {e}") | ||
| data = {} |
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 more granular error handling for UNESCO API requests.
Wrapping each API request in its own try/except block would allow the function to return available data even if some requests fail.
Suggested change
| try: | |
| for code, name in indicators.items(): | |
| url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json" | |
| response = requests.get(url, timeout=10) | |
| response.raise_for_status() | |
| data[name] = response.json()['dataSets'][0]['series'] | |
| except requests.exceptions.RequestException as e: | |
| flash(f"Could not retrieve data from UNESCO API: {e}") | |
| data = {} | |
| for code, name in indicators.items(): | |
| url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json" | |
| try: | |
| response = requests.get(url, timeout=10) | |
| response.raise_for_status() | |
| data[name] = response.json()['dataSets'][0]['series'] | |
| except requests.exceptions.RequestException as e: | |
| flash(f"Could not retrieve data for '{name}' from UNESCO API: {e}", "warning") |
Comment on lines
+74
to
+79
| {% with messages = get_flashed_messages(with_categories=true) %} | ||
| {% if messages %} | ||
| {% for category, message in messages %} | ||
|
{{ message }}
|
||
| {% endfor %} | ||
| {% endif %} |
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: Flashed message categories may not map directly to Bootstrap alert classes.
If your flash categories differ from Bootstrap's expected values, map them accordingly to ensure correct alert styling.
Suggested change
| {% with messages = get_flashed_messages(with_categories=true) %} | |
| {% if messages %} | |
| {% for category, message in messages %} | |
| <div class="alert alert-{{ category }}">{{ message }}div> | |
| {% endfor %} | |
| {% endif %} | |
| {% set bootstrap_alert_map = { | |
| 'error': 'danger', | |
| 'message': 'info', | |
| 'info': 'info', | |
| 'success': 'success', | |
| 'warning': 'warning', | |
| 'danger': 'danger' | |
| } %} | |
| {% with messages = get_flashed_messages(with_categories=true) %} | |
| {% if messages %} | |
| {% for category, message in messages %} | |
| <div class="alert alert-{{ bootstrap_alert_map.get(category, 'info') }}">{{ message }}div> | |
| {% endfor %} | |
| {% endif %} |
|
|
||
| if __name__ == '__main__': | ||
| app.run(debug=True) | ||
| app.run(host='0.0.0.0', port=8080, debug=True) |
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.
issue (security): Running with debug=True in production is a security risk.
Use environment variables or configuration files to manage the debug setting and ensure it is disabled in production.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.