Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

preserve user-specified report source instead of config default #1070

Merged
merged 3 commits into from
Jan 8, 2025

Conversation

alesaccoia
Copy link
Contributor

The report_source parameter was being incorrectly overridden by the config default value due to Python's 'or' operator behavior with truthy values. Changed the logic to explicitly check for the passed parameter first before falling back to config values.

The report_source parameter was being incorrectly overridden by the config 
default value due to Python's 'or' operator behavior with truthy values. 
Changed the logic to explicitly check for the passed parameter first before 
falling back to config values.
Using None as default allows for proper validation of unspecified sources and follows the principle of explicit configuration.
@lmontin
Copy link

lmontin commented Jan 8, 2025

I tested the change and it worked. ie when using the nextjs ( port 3000 ) interface my preferences selection for report source My Documents ( local ) was respected

Add more descriptive logging statements when loading and processing local documents: count of loaded documents
@alesaccoia
Copy link
Contributor Author

Cool. Just added some logging for Local mode in the last commit

Copy link
Owner

@assafelovic assafelovic left a comment

Choose a reason for hiding this comment

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

This is super helpful and important bug fix thank you @alesaccoia !

@assafelovic assafelovic merged commit 1e34dd9 into assafelovic:master Jan 8, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants