chore(pre-commit): add pre-commit hook to enforce llama_stack logger usage

[Elbehery]

What does this PR do?

This PR adds a step in pre-commit to enforce using llama_stack logger.

Currently, various parts of the code base uses different loggers. As a custom llama_stack logger exist and used in the codebase, it is better to standardize its utilization.

This PR contains two commits

• commit add a hook in pre-commit to check that llama_stack custom logger is being used, otherwise it fails.
• commit which replaces all other logger used in the current codebase with llama_stack logger.

Replaces #2868
Part of #2865

cc @leseb @rhuss

[rhuss]

Thanks for the update!

Can you please check the section called "Unchanged files with check annotations" at the end of this PR's files list? It appears that some violations still occur solely due to the import logging. You should add the proper annotation for those lines as well if the usage of logging in those files are justified. Ideally all those warnings should be gone.

[rhuss]

I'm referring to those files:

[Elbehery]

Thanks for the update!

Can you please check the section called "Unchanged files with check annotations" at the end of this PR's files list? It appears that some violations still occur solely due to the import logging. You should add the proper annotation for those lines as well if the usage of logging in those files are justified. Ideally all those warnings should be gone.

thanks so much for your review 🙏🏽

I have already checked actually :), the import is needed for this func

    def _remove_root_logger_handlers(self):
        """
        Remove all handlers from the root logger. Needed to avoid polluting the console with logs.
        """
        root_logger = logging.getLogger()

        for handler in root_logger.handlers[:]:
            root_logger.removeHandler(handler)
            logger.info(f"Removed handler {handler.__class__.__name__} from root logger")

i tried to replace logging.getLogger() with logger., but it is not possible

adding the comment though 👍🏽

[Elbehery]

fixed 👍🏽

[rhuss]

i tried to replace logging.getLogger() with logger., but it is not possible

I understand. But please can you add to all the imports which require direct import of logging the comment, that passes the checks ? There are still 10 warnings left, please see the files at the end of https://github.com/meta-llama/llama-stack/pull/3061/files

I've mentioned it already in #3061 (comment) : For any usage when you need that import you have to add the comment to allow it.

[Elbehery]

i tried to replace logging.getLogger() with logger., but it is not possible

I understand. But please can you add to all the imports which require direct import of logging the comment, that passes the checks ? There are still 10 warnings left, please see the files at the end of https://github.com/meta-llama/llama-stack/pull/3061/files

I've mentioned it already in #3061 (comment) : For any usage when you need that import you have to add the comment to allow it.

thanks for your review :)

It seems that the view mentioned on #3061 (comment), is only available for reviewers, as I have been through the PR (files and checks) many time and can not see the same warning :)

But I should have checked all import logging in the code and added the annotation, will do now 👍🏽

[Elbehery]

rebased 👍🏽

@mattf hello ✋🏽

since this has been already approved 3 days ago, any other changes needed here ?

cc @ashwinb

[Elbehery]

@mattf hello ✋🏽

I dont know why it did not merge, as it was approved yesterday !!

[Elbehery]

rebased 👍🏽

[mattf]

@Elbehery merge in main so we can get pre-commit passing

[Elbehery]

@Elbehery merge in main so we can get pre-commit passing

sorry just to be sure, so i do not rebase, but merge main into my branch

correct ?

[Elbehery]

thanks so much @mattf 🙏🏽

can we merge this now ?