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

Use logging instead of printing #58

Open
vsoch opened this issue Jun 27, 2021 · 6 comments
Open

Use logging instead of printing #58

vsoch opened this issue Jun 27, 2021 · 6 comments

Comments

@vsoch
Copy link
Contributor

vsoch commented Jun 27, 2021

Heyo! I'm wondering if instead of having a bunch of print statements (that the user cannot control)

Retrieving repository info for LLNL/b-mpi3
Checking GitHub API token... Token validated.
Auto-retry limit for requests set to 10.
Reading '/home/vanessa/Desktop/Code/contributor-ci/contributor_ci/main/extractors/repos/repos-info.gql' ... File read!
Sending GraphQL query...
Checking response...
HTTP STATUS 200 OK
API Status {"limit": 5000, "remaining": 4998, "reset": 1624819230}
Data received!

It would be possible and make sense to using logging instead, so it can be made quiet? For my user case, I have a command that hits a few API endpoints and then needs to pipe to file, and I'm not able to control this output.

@IanLee1521
Copy link
Member

I think this is a great idea! No objections from me.

Is there any chance you'd be willing to work this in to a pull request that we could review and merge?

@LRWeber
Copy link
Member

LRWeber commented Jun 28, 2021

FWIW, there is actually a verbosity parameter included in the query methods. e.g.:

def queryGitHubFromFile(self, filePath, gitvars=None, verbosity=0, **kwargs):

verbosity (Optional[int]): Changes output verbosity levels.
If < 0, all extra printouts are suppressed.
If == 0, normal print statements are displayed.
If > 0, additional status print statements are displayed.
Defaults to 0.

@leebrian
Copy link
Collaborator

I think a PR with these updates would be nice.

I’ve gotten around needing this by just piping the output from the process to whatever log file I like, and using the verbosity param that LRWeber points out.

@vsoch
Copy link
Contributor Author

vsoch commented Jun 28, 2021

Setting verbosity to -1 still can't control the print statements:

$ cci cfa --terminal https://github.com/vsoch/salad 
Generating CFA for https://github.com/vsoch/salad
Stored new data file path '/home/vanessa/Desktop/Code/contributor-ci/.cci/data/latest/cci-repos.json'
Importing existing data file '/home/vanessa/Desktop/Code/contributor-ci/.cci/data/latest/cci-repos.json' ... Imported!

Retrieving repository info for vsoch/salad
Checking GitHub API token... Token validated.
Auto-retry limit for requests set to 10.
# nothing above this line should be printed!
---
repository: vsoch/salad
title: vsoch/salad
---

@vsoch
Copy link
Contributor Author

vsoch commented Jun 28, 2021

Would y'all be okay with removing these custom print functions in favor of standard python logging? It looks like you use it in other parts of the library but they didn't make it here! Do you remember if there was a special reason to do this?

@LRWeber
Copy link
Member

LRWeber commented Jun 28, 2021

The primary reason for using the basic print rather than logging was to enable some of the more human-friendly feedback for when the commands are run directly in real-time, for example the visible countdown when waiting to retry a query (especially when the GitHub API limit triggers a much longer wait time).

In short, this update will mean more extensive re-working of how some of the info is presented. However, if we believe the tradeoff is worthwhile, I'm sure we can come up with a similarly informative logging friendly version.

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

No branches or pull requests

4 participants