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

Adding message to run/rerun, other interface tweaks #32

Merged
merged 7 commits into from May 30, 2020
Merged

Conversation

vsoch
Copy link
Owner

@vsoch vsoch commented May 29, 2020

This pull request will address the following issues:

vsoch added 2 commits May 29, 2020 14:48
…nd print of host/port to terminal on qme start

Signed-off-by: vsoch <vsochat@stanford.edu>
Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch
Copy link
Owner Author

vsoch commented May 29, 2020

@yarikoptic please review! I tried to just add you as a reviewer to the repo but it didn't give me the option, I suppose I forgot how to do that :P

Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch
Copy link
Owner Author

vsoch commented May 29, 2020

Just added QME_MESSAGELEVEL to control the logging level.

@yarikoptic
Copy link
Contributor

Invite me as a collaborator

@yarikoptic
Copy link
Contributor

Consider using common prefix for all env var related to logging, eg shorter QME_LOG_. Then you could uniformly add more settings as needed. I will also appreciate it personally since that is the convention I used in a good number of projects

@vsoch
Copy link
Owner Author

vsoch commented May 29, 2020

Good idea @yarikoptic, but we can add that in a separate PR when there is some refactor of the logger (out of scope here).

Copy link
Contributor

@yarikoptic yarikoptic left a comment

Choose a reason for hiding this comment

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

Left some comments, which might be irrelevant but might be addressed for perfection ;-)

qme/app/server.py Outdated Show resolved Hide resolved
qme/main/database/relational.py Outdated Show resolved Hide resolved
Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch vsoch mentioned this pull request May 30, 2020
@vsoch
Copy link
Owner Author

vsoch commented May 30, 2020

@yarikoptic ready again for review! For the logging I can do that in another PR.

@yarikoptic
Copy link
Contributor

Left a follow up question to one comment which you marked resolved

Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch
Copy link
Owner Author

vsoch commented May 30, 2020

Sorry that I missed the bit about the host - it's added now! :)

@yarikoptic
Copy link
Contributor

Sorry to be the pain and not properly describing my intentions, I should have had add a suggestion but ATM hard on the phone.

Before using host variable, don't you need to move it's final definition there? (And use also below), ie

host=host or QME_HOSTNAME

Signed-off-by: vsoch <vsochat@stanford.edu>
@vsoch
Copy link
Owner Author

vsoch commented May 30, 2020

Oversight on my part - fixed now!

Copy link
Contributor

@yarikoptic yarikoptic left a comment

Choose a reason for hiding this comment

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

Thank you! Cheers

Signed-off-by: vsoch <vsochat@stanford.edu>
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.

None yet

2 participants