btlogging/loggingmachine.py: Fix bw compat API.

[mvds00]

The bt logging mechanism breaks regular logging in various ways, that are solved by this commit.

Bug

Logging from outside of bittensor is broken, for instance, retry logs using:

logger.warning('%s, retrying in %s seconds...', e, _delay)

and this will output the literal %s, as the args are hijacked by the attempt at bw compatibility in bittensor, because the args e and _delay would land on kwargs prefix and suffix.

Also, all logging originates quite visibly from loggingmachine.py, which, in a literal sense, is correct, but not very helpful information.

Description of the Change

• add stacklevel=2 so that the filename of the originating call is logged (requires Python >= 3.8)
• move *args before prefix and suffix kwargs so regular logging calls using args, such as in retry, still work as intended
• additional commit to get rid of the " - " prefixes and suffixes that are added when prefix and/or suffix are not used
• handles an issue in CPython < 3.11 with logging.exception()

Alternate Designs

Reworking the logging entirely is also a possibility. But this seemed to be the easy fix.

Possible Drawbacks

Any code relying on prefix and suffix also being positional arguments, is now broken. This is intentional.

Verification Process

I ran code that relies on bittensor and btlogging to verify that logging is now better.

Release Notes

• Fixed and improved logging

[basfroman]

I recommend using sys.implementation and sys.version_info for checking the Python implementation and version instead of platform. This approach is more direct, avoids extra imports, and improves performance by leveraging attributes directly available in the sys module, which is standard and efficient for such checks.

Changes:

if (sys.implementation.name == "cpython" and
    sys.version_info.major == 3 and
    sys.version_info.minor < 11):
    # do something

[mvds00]

Done. Syntax is a bit different but as suggested by CI.

[basfroman]

Perfect. Also pls add unit tests related with new method and updated ones.

[mvds00]

You mean tests for the logging functionality? They are not part of CI but they all pass. Of the CI tests two tests failed but they seem unrelated: "get current emissions and validate that Alice has gotten tao" and "assert updated_axon.ip == external_ip", could it be that something else is off there? I just pushed a unit test that checks the fixed logging elements.

[basfroman]

My personal recommendation is to use the following command before sending a PR:
clear && python3 -m ruff format . && mypy --ignore-missing-imports bittensor/ --python-version=3.9 && mypy --ignore-missing-imports bittensor/ --python-version=3.10 && mypy --ignore-missing-imports bittensor/ --python-version=3.11 && flake8 bittensor/ --count

I'm surprised that your implementation affected and broke tests that were not related to this. Let's try the following:

• merget staging to your PR (I see This branch is out-of-date with the base branch below);
• use locally the command I indicated above;

I expect there will be no broken tests.

[basfroman]

@mvds00, @heeres pls fix ruff complaint using command ruff format . in your env

[mvds00]

@mvds00, @heeres pls fix ruff complaint using command ruff format . in your env

Why? We didn't touch the files ruff is complaining about. I think this commit is the culprit:

commit f02c115673d10ca923e94308448e66102a1c11d6
Author: Benjamin Himes <benhimes@opentensor.dev>
Date:   Mon Jul 22 15:39:10 2024 +0200

...

 KEY_NONCE: Dict[str, int] = {}
 
-#######
-# Monkey patch in caching the convert_type_string method
-#######
-if hasattr(RuntimeConfiguration, "convert_type_string"):
-    original_convert_type_string = RuntimeConfiguration.convert_type_string
-
-    @functools.lru_cache(maxsize=None)
-    def convert_type_string(_, name):
-        return original_convert_type_string(name)
-
-    RuntimeConfiguration.convert_type_string = convert_type_string
-#######
 
 
 class ParamWithTypes(TypedDict):

Because the removal of the comments and code makes the newlines form three empty lines which is obviously Very Bad.

How could it have come through CI?

[basfroman]

@mvds00, @heeres pls fix ruff complaint using command ruff format . in your env

Why? We didn't touch the files ruff is complaining about. I think this commit is the culprit:

commit f02c115673d10ca923e94308448e66102a1c11d6
Author: Benjamin Himes <benhimes@opentensor.dev>
Date:   Mon Jul 22 15:39:10 2024 +0200

...

 KEY_NONCE: Dict[str, int] = {}
 
-#######
-# Monkey patch in caching the convert_type_string method
-#######
-if hasattr(RuntimeConfiguration, "convert_type_string"):
-    original_convert_type_string = RuntimeConfiguration.convert_type_string
-
-    @functools.lru_cache(maxsize=None)
-    def convert_type_string(_, name):
-        return original_convert_type_string(name)
-
-    RuntimeConfiguration.convert_type_string = convert_type_string
-#######
 
 
 class ParamWithTypes(TypedDict):

Because the removal of the comments and code makes the newlines form three empty lines which is obviously Very Bad.

How could it have come through CI?

I guess know what is happening. We have extra line which is the reason for ruff complaint. This PR should to fix that. After I merge it, you need to update your branch from staging and all the checkouts.

[mvds00]

I rebased against staging after the ruff issue was fixed.

[basfroman]

Thank you for helping me discover the issue!

[basfroman]

pls return back e2e test and e2e utils.

[mvds00]

pls return back e2e test and e2e utils.

Why? The e2e test I removed refers to a function that was removed from subtensor, so it will always fail. The e2e utils contains a bug in epoch calculation, probably causing the weight setting test to fail. So without these fixes it will never pass CI.

[basfroman]

pls return back e2e test and e2e utils.

Why? The e2e test I removed refers to a function that was removed from subtensor, so it will always fail. The e2e utils contains a bug in epoch calculation, probably causing the weight setting test to fail. So without these fixes it will never pass CI.

Now there is a process of reorganizing the nodes on the subtensor side. This will be up and running soon for testing and the tests will be successful.

[thewhaleking]

How are the majority of changes to this file relevant to this PR?

[mvds00]

They are needed to fix issues in CI. I encountered these issues while analyzing the possible reasons why the original PR didn't come through CI. Should I change the text of the original PR?

[thewhaleking]

So the general rule is "1 PR should do 1 thing". Sometimes that "1 thing" can be large, sometimes small. Are these changes needed to fix the CI only as it relates to what this PR also fixes, or are they more general? They appear to be more general, from my looking.

[mvds00]

They are more general indeed. Wasn't aware of the PR rule - I only split it at the commit level. Shall I split them into 3 or 4 PRs?

[thewhaleking]

No worries. I would probably just do two. The test_swap_hotkey.py file will fail until it is fixed on the subtensor side, but just don't worry about it for the moment.

[ibraheem-abe]

@mvds00 Can you please update the branch with base staging? Thank you!

[mvds00]

@mvds00 Can you please update the branch with base staging? Thank you!

Just did, and automated the process towards the future.