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

Flask Debug Toolbar #120

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

Javiercerna
Copy link

@Javiercerna Javiercerna commented Oct 6, 2021

I'll skip the PR template for now, since this is just a draft PR to ask questions.

When it's finished, it should close: #73

SQLALCHEMY_TRACK_MODIFICATIONS = False
BASE_URL = "https://openqairamapnapi.qairadrones.com/"
DEBUG = False
SECRET_KEY = "my-secret-key" # TODO: Replace with a more secure key, and don't push it to Github
Copy link
Author

Choose a reason for hiding this comment

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

This is my first question, @GersonMontesinos. This secret key shouldn't be written here even if it's open source, for security reasons. One alternative would be to do this:

Suggested change
SECRET_KEY = "my-secret-key" # TODO: Replace with a more secure key, and don't push it to Github
SECRET_KEY = os.environ.get("SECRET_KEY")

But then in your production environment you would need to set this environment variable. How would you like to proceed?

@@ -75,7 +75,7 @@ def getQhawaxStatus():
name = request.args.get("name")
try:
return (
str(same_helper.getQhawaxStatus(name))
make_response(jsonify(same_helper.getQhawaxStatus(name)))
Copy link
Author

@Javiercerna Javiercerna Oct 6, 2021

Choose a reason for hiding this comment

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

This is my second question, @GersonMontesinos. If I keep these endpoints as they are, they return a str, which will then become a HTML without any <body> tags. The toolbar can only be used for HTML responses with a <body> tag or with JSON responses (now that I added support for them).

There are 3 options here:

  1. Do nothing, and you will change this later at some point. The toolbar won't work for these endpoints until you fix it.
  2. Change it to a JSON string, with the code that I wrote here.
  3. Change it to a "valid" HTML, by including the missing tags. It would be something like this:
Suggested change
make_response(jsonify(same_helper.getQhawaxStatus(name)))
<html><body><pre>{}</pre></body></html>'.format(
same_helper.getQhawaxStatus(name)
)

For both options 2 and 3, I would have to change it for all your non-JSON responses, which could take some time. And, more important, your API customers (even the frontend) might need to modify their code in case they are expecting a str() instead of a JSON str or a "valid" HTML (with the extra tags).

What would you like to do? 😄

@Javiercerna Javiercerna marked this pull request as draft October 6, 2021 19:31
@GersonMontesinos
Copy link
Contributor

Hey @Javiercerna :

Answering to the first comment: why are the following lines of code are added? is it a requirement for the flask debug toolbar to work?
DEBUG = False SECRET_KEY = "my-secret-key" # TODO: Replace with a more secure key, and don't push it to Github
The API has been working without having to use one.

Answering to the second comment: that endpoint and the others that give a string response are only used by the IoT devices and some by the frontend. I would like to make sure that if these endpoints - if left unchanged - could cause any trouble when using the toolbar for another endpoints.

@Javiercerna
Copy link
Author

Hey @Javiercerna :

Answering to the first comment: why are the following lines of code are added? is it a requirement for the flask debug toolbar to work? DEBUG = False SECRET_KEY = "my-secret-key" # TODO: Replace with a more secure key, and don't push it to Github The API has been working without having to use one.

Answering to the second comment: that endpoint and the others that give a string response are only used by the IoT devices and some by the frontend. I would like to make sure that if these endpoints - if left unchanged - could cause any trouble when using the toolbar for another endpoints.

  1. Yes, the two variables (DEBUG and SECRET_KEY) are needed for the toolbar, I haven't found a workaround.
  2. I didn't understand what you answered. Could you elaborate a bit more, please?

@Javiercerna
Copy link
Author

Javiercerna commented Oct 7, 2021

Hey @Javiercerna :
Answering to the first comment: why are the following lines of code are added? is it a requirement for the flask debug toolbar to work? DEBUG = False SECRET_KEY = "my-secret-key" # TODO: Replace with a more secure key, and don't push it to Github The API has been working without having to use one.
Answering to the second comment: that endpoint and the others that give a string response are only used by the IoT devices and some by the frontend. I would like to make sure that if these endpoints - if left unchanged - could cause any trouble when using the toolbar for another endpoints.

  1. Yes, the two variables (DEBUG and SECRET_KEY) are needed for the toolbar, I haven't found a workaround.
  2. I didn't understand what you answered. Could you elaborate a bit more, please?

Maybe you are asking if the toolbar won't work at all if we don't change the endpoints that return str? If we don't change them, the toolbar won't work for those endpoints, but it will still work fine for the endpoints that return JSON (and any endpoints that return HTML, in case you have them)

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.

What cannot be measured, cannot be improved.
2 participants