-
Notifications
You must be signed in to change notification settings - Fork 27
[mock_uss] Move mock_uss webapp to app module #1219
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
[mock_uss] Move mock_uss webapp to app module #1219
Conversation
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.
LGTM
@@ -1,6 +0,0 @@ | |||
from monitoring.mock_uss import SERVICE_MESSAGESIGNING, config |
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 removal intended? Is it because the check made here is unnecessary?
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.
Actually, it was removed because SERVICE_MESSAGESIGNING is no longer as easily accessible and it didn't seem worthwhile to adapt this check, but that functionality change probably should probably be in a separate PR; I've re-added the check without the constant.
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.
Ah, and now I'm recalling the other reason: Config.CERT_BASE_PATH doesn't exist, nor does ENV_KEY_CERT_BASE_PATH -- basically, this code is broken.
webapp = MockUSS(__name__) | ||
webapp.secret_key = os.environ.get("SECRET_KEY") or os.urandom(24) | ||
if os.environ.get("MOCK_USS_PROXY_VALUES"): | ||
values = os.environ.get("MOCK_USS_PROXY_VALUES", "").split(",") |
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.
FTR: checked this change
This PR takes a small step toward #1218 by placing the actual
webapp
Flask server in app.py rather than init.py. This does not resolve the issue as other __init__s still automatically pull inwebapp
, but this is a start.