Skip to content

Conversation

@gordonwatts
Copy link
Collaborator

This PR will exchange some of the rich.print calls that happen as the library runs and route them through logging.info and logging.warning and logging.error.

  • Less inline output while library is running
  • Allows user of library to control how much output is printed
  • Brings it into line for the "correct" way to use library, according to python.

@gordonwatts gordonwatts self-assigned this May 3, 2024
@gordonwatts gordonwatts added the enhancement New feature or request label May 3, 2024
@gordonwatts gordonwatts requested a review from BenGalewsky May 3, 2024 00:46
@codecov
Copy link

codecov bot commented May 3, 2024

Codecov Report

Attention: Patch coverage is 72.41379% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 74.97%. Comparing base (5bc25f1) to head (b0d1b00).
Report is 4 commits behind head on 3.0_develop.

Files Patch % Lines
servicex/query.py 82.60% 4 Missing ⚠️
servicex/servicex_client.py 33.33% 4 Missing ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##           3.0_develop     #371      +/-   ##
===============================================
+ Coverage        73.78%   74.97%   +1.19%     
===============================================
  Files               42       42              
  Lines             2300     2406     +106     
===============================================
+ Hits              1697     1804     +107     
+ Misses             603      602       -1     
Flag Coverage Δ
unittests 74.97% <72.41%> (+1.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator Author

@gordonwatts gordonwatts left a comment

Choose a reason for hiding this comment

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

There is a chance that some of these logging.info should be downgraded to logging.debug.

@gordonwatts gordonwatts marked this pull request as ready for review May 3, 2024 00:50
return self

async def submit_and_download(
self, signed_urls_only: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

Formatting changes in the same commit as the code changes make the review a lot harder. If you want to change formatting, it's more polite to make that a single commit and then have the meat of the PR in a second commit

Copy link
Contributor

@BenGalewsky BenGalewsky left a comment

Choose a reason for hiding this comment

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

Could you make the logger class variables in these two classes? It seems messy to keep repeating the logging.get_logger("__name__")

@BenGalewsky BenGalewsky merged commit 6d04eb8 into 3.0_develop May 3, 2024
@BenGalewsky BenGalewsky deleted the feat/use_python_logging branch May 3, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants