Update: Printing to logging messages #3313

[PredictiveManish]

Description This PR updates all the print() in augur/tasks to logging which helps in better formatting and makes it helpful in unifying logging in augur project. As mentioned in #3313

This PR fixes #3313

Notes for Reviewers

Signed commits

• [x] Yes, I signed my commits.

[PredictiveManish]

This seems like a good start! I'd wonder if there are ways to use existing logger objects or create them in more similar ways to how the ones that already exist are created. Right now it sorta feels like there are more new loggers to have to manage (that said though, i havent gotten deep into how augur does logging)

Wherever logger were absent in files I created and where already defined i used them, but in some files each methods have mentioned there own logger, I will try to reduce unnecessary loggers.

[PredictiveManish]

@PredictiveManish : Can you address @MoralCode 's suggestion?

Yes sir, following what he said.

[PredictiveManish]

@MoralCode Sorry for this question, but if possible can you make a small explanation what it means by tech debt? I can't understand what does this means.

[MoralCode]

@MoralCode Sorry for this question, but if possible can you make a small explanation what it means by tech debt? I can't understand what does this means.

I'll probably do a worse job explaining than other sources online, but Tech debt is essentially a nickname for "technical debt", which is essentially a software engineering/developer term for code "mess" that accumulates over time. For example, if part of a codebase (such as a function to insert bulk data into a database, or a logging system) is refactored/rewritten, but not everything that was using the old system got moved over, the code now has two methods for doing that thing. You could also think of technical debt as maybe "shortcuts" that get taken with regard to the code (often done to save time) that need to be cleaned up later. Sort of like "hey ill just do things this way temporarily and fix it later". Its called debt because its similar to "taking out a loan against your future" but for code. Just like how you may get a student loan to pay for university and pay it back later when you get a job, tech debt is similar - you are doing something now that requires some of your time in the future to clean up or do properly.

Augur as a codebase started as an academic project (so lots of experimenting was happening) and has also been around for a while and has been through several rewrites. The deeper i dig into the code tha more stuff i find that feels like tech debt (i.e. two functions doing the same things, two different ways to print information to the console - which this PR helps with, etc).

Ive been labeling issues that identify tech debt and PRs that help clean up that debt and have made it somewhat of my personal mission, especially this month, to see how much of Augur's tech debt I can clean out. This will:

• help make it so that its much easier for new people to contribute to and understand the code
• help me learn the codebase because I'll likely be looking through every part of it
• reduce the amount of code i need to change for bigger future projects in the upcoming year or so (such as getting gitlab collection working, adding support for Forgejo, etc.)

Hope that helps!

[PredictiveManish]

@MoralCode Sorry for this question, but if possible can you make a small explanation what it means by tech debt? I can't understand what does this means.

I'll probably do a worse job explaining than other sources online, but Tech debt is essentially a nickname for "technical debt", which is essentially a software engineering/developer term for code "mess" that accumulates over time. For example, if part of a codebase (such as a function to insert bulk data into a database, or a logging system) is refactored/rewritten, but not everything that was using the old system got moved over, the code now has two methods for doing that thing. You could also think of technical debt as maybe "shortcuts" that get taken with regard to the code (often done to save time) that need to be cleaned up later. Sort of like "hey ill just do things this way temporarily and fix it later". Its called debt because its similar to "taking out a loan against your future" but for code. Just like how you may get a student loan to pay for university and pay it back later when you get a job, tech debt is similar - you are doing something now that requires some of your time in the future to clean up or do properly.

Augur as a codebase started as an academic project (so lots of experimenting was happening) and has also been around for a while and has been through several rewrites. The deeper i dig into the code tha more stuff i find that feels like tech debt (i.e. two functions doing the same things, two different ways to print information to the console - which this PR helps with, etc).

Ive been labeling issues that identify tech debt and PRs that help clean up that debt and have made it somewhat of my personal mission, especially this month, to see how much of Augur's tech debt I can clean out. This will:

• help make it so that its much easier for new people to contribute to and understand the code
• help me learn the codebase because I'll likely be looking through every part of it
• reduce the amount of code i need to change for bigger future projects in the upcoming year or so (such as getting gitlab collection working, adding support for Forgejo, etc.)

Hope that helps!

Actually thanks so much for this explanation, and you had done great job with this explanation as it was easier to understand then a blog written in high key words. Thanks again! I hope I learn more things from you with time!

Also I will make sure I do help you anything you want I'm learning things but I'll do my best, wherever you see any recursive work is there and it can be outsourced just tag me, I'll surely help and make this software better.

[PredictiveManish]

@MoralCode the testcase which is failing is showing that in some files else is used after return and that's where around 9-10 warnings are there which is failing run-linting-checks/runner/pylint (pull request) do you suggest upgrading that?

[MoralCode]

@MoralCode the testcase which is failing is showing that in some files else is used after return and that's where around 9-10 warnings are there which is failing run-linting-checks/runner/pylint (pull request) do you suggest upgrading that?

If you added the else: in your PR, i would say yes it should be fixed. A quick skim of the diff here seems to suggest these were present before though, so I wouldn't worry about it. Code changes like that are more of a stylistic choice anyway and I think keeping the else - while not the most concise - may make the code more readable.

In other words, theres a solid chance the pylint rules could be "wrong". dont sweat it.

[PredictiveManish]

@MoralCode the testcase which is failing is showing that in some files else is used after return and that's where around 9-10 warnings are there which is failing run-linting-checks/runner/pylint (pull request) do you suggest upgrading that?

If you added the else: in your PR, i would say yes it should be fixed. A quick skim of the diff here seems to suggest these were present before though, so I wouldn't worry about it. Code changes like that are more of a stylistic choice anyway and I think keeping the else - while not the most concise - may make the code more readable.

In other words, theres a solid chance the pylint rules could be "wrong". dont sweat it.

Yes, I didn't added them, ok thanks for clarifying that pylint errors is not an issue, i thought it'll cause trouble in merging to main. Do you've any other suggestion for the PR, otherwise I think it's resolving the issue and replaced all print statement to logger command.

[MoralCode]

I think this is ready but im holding off on marking it as such until we get the next release cut

[PredictiveManish]

I think this is ready but im holding off on marking it as such until we get the next release cut

Ok sir.

[MoralCode]

oops, i should probably test this soon

[PredictiveManish]

What reviewdog is choking on seems pedantic. If @PredictiveManish could fix it I think we'd have it passing. I technically agree that:

if "joe" == "bob": 
    return whatever

Do next thing

is better than:

if "joe" == "bob": 
    return whatever
else: 
    Do next thing

But good golly it works either way.

yeah sure, I'm fixing this. will do it in 24 hours sir as I have an exam today.

[sgoggins]

@MoralCode : / @PredictiveManish : This one has a merge conflict that's emerged. I'm uncertain if that's why CII is failing.