Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

KeyError: "Attempt to overwrite 'args' in LogRecord" #271

Closed
kornicameister opened this issue May 21, 2020 · 11 comments
Closed

KeyError: "Attempt to overwrite 'args' in LogRecord" #271

kornicameister opened this issue May 21, 2020 · 11 comments
Labels
enhancement Improvement to an already existing feature

Comments

@kornicameister
Copy link
Contributor

kornicameister commented May 21, 2020

Expected result:

I am able to use args as keyword to denote argument to string interpolation when using loguru

Actual result:

Opposite of above ;-).In fact it seems that if you try to specify whatever is originally available in Record from logging of python, python will think you are crazy to put such things in record.

Facts

  • loguur.__version__ = 0.5.0
  • python --version = Python 3.7.6
  • lsb_release -sd = Linux Mint 19.3 Tricia
  • context = application

Reproduce

git clone https://github.com/kornicameister/axion.git /tmp/axion --depth 1 ; \
cd /tmp/axion ; \
git checkout c9d85e76bd48719aab7edded1d4287a52ecb145e \
command -v tox && tox -e "py37-test" -- -k test_not_empty_signature || echo "need tox" ; \
cd - \
rm -rf /tmp/axion

Root cause

Relevant line that causes issue available here

Alternatively, it might be caused by conftest#caplog here, but this is unlikely given that only one test fails ;-)

@Delgan
Copy link
Owner

Delgan commented May 22, 2020

Hi @kornicameister, thanks for the detailed bug report!

Since version 0.5.0 of Loguru, keyword arguments when calling a logging function are automatically added to the extra dict. This feature has been requested multiple times to facilitate structured logging.

I didn't think it could have that side effect. This happens because if you use a handler from the standard logging library, the extra dict is used to update the LogRecord. This happens also if logger.bind(name="name") is used for example.

Unfortunately there's not much I can do. While using a standard Handler, one have to be careful not to use "forbidden" keywords. Possible workarounds:

  • Rename args to args_
  • Use f-strings to format the message
  • Specify logger.opt(capture=False)

Also, I noticed in your code base you're using opt(record=True) but I'm not sure you need it if you don't use logger.info("Current thread: {record[thread]}") for example.

@kornicameister
Copy link
Contributor Author

kornicameister commented May 22, 2020

@Delgan so there's no way to..hmm..., filter such keywords right inside of particular handler?
Right before they go any way deeper?


Apart misuse in my codebase, would you mind elaborating in my repository? I a little bit intrigued but wouldn't want to make an offtopic here, if you don't mind.

@Delgan
Copy link
Owner

Delgan commented May 22, 2020

so there's no way to..hmm..., filter such keywords right inside of particular handler?
Right before they go any way deeper?

Hum... I prefer not to automatically strip out keywords because I think it risks to just shift the problem away (people might wonder where the expected values have gone).

However, instead of using LogRecord(..., extra=extra) I could probably create the instance with LogRecord(..., extra={"extra": extra}). The Loguru's extra dict will no longer automatically populate LogRecord attributes but will stay accessible with LogRecord.extra. I don't think this will result in usability loss, and on the contrary this will prevent users to be bitten by "Attempt to overwrite" errors. 🤔

Apart misuse in my codebase, would you mind elaborating in my repository? I a little bit intrigued but wouldn't want to make an offtopic here, if you don't mind.

Do you want me to open a new ticket in your repository to discuss loguru usage being made?

@Delgan Delgan added the enhancement Improvement to an already existing feature label May 22, 2020
@kornicameister
Copy link
Contributor Author

kornicameister commented May 22, 2020

Hum... I prefer not to automatically strip out keywords because I think it risks to just shift the
problem away (people might wonder where the expected values have gone).

That's fair point. I wouldn't want that either.

The Loguru's extra dict will no longer automatically populate LogRecord attributes but will stay
accessible with LogRecord.extra.

That makes sense, if a point is just to store it for further use and that's rather internal detail, it should be quite safe to provide such change.

Another useful feature/chage, can be developing a little mypy or flake plugin, although I'd say mypy and its hinting of f-string makes more sense. Just a thought, but quite intriguing that could prevent users from putting arguments into their messages without counterparts in extras. TBH you could even take this further with extension in type toward Callable if there is a lazy=True for opt set but latter can be hard to implement if logger.opt(lazy=true) does not happen on same line or something. Just a thought for feature ;-), if you want :)

Do you want me to open a new ticket in your repository to discuss loguru usage being made?

Only if you want and have time to do that. I will probably look into it myself sometime in near future. I had a brief though I might be misusing loguru because of something I read in docs and is interpreted in wrong way. On the other hand, I wanted to avoid discussing about "me" and my codebase here to focus on loguru and those args :D

@Delgan
Copy link
Owner

Delgan commented May 22, 2020

There exists type hints for loguru, anyone should be able to benefit from it using mypy, right?

I opened kornicameister/axion#328 to discuss usage of loguru in your project. :)

@kornicameister
Copy link
Contributor Author

kornicameister commented May 22, 2020

type hints do say say that you can logger.debug with first argumen being str and rest of arguments is of typing.Any. And that's fine because that's pretty much all you can tell to mypy or similar tool about method signature 'cause everything else that you put in those messages are kwargs that are well...typing.Any.

My idea was something along this line:

logger.opt(..).debug(
  '{a} / {b} = {c}, # Error: Message defines "c" placeholder but it is missing in extras, found [a, b] though
  a=4,
  b=2,
)

or

logger.opt(..).debug(
  '{a} * {b} = {c}, 
  a=4,
  b=2,
  c=8,
  d=16,  # Warning: Message does not define d variable
)

or

logger.opt(..).info(
  'My name is {n}', 
  n=lamba _: "kornicameister",  # Error: Expected Callable[[], typing.Any], got Callable[[typing.Any], typing.Any]
)

what about that 🍭 :D

@Delgan
Copy link
Owner

Delgan commented May 22, 2020

Oh, I see! Sure, that would be neat. But I don't think that's possible knowing the limitations of Python type hints. Maybe the problem can be circumvented with a mypy plugin but that's really outside my area of knowledge. 😁

@kornicameister
Copy link
Contributor Author

kornicameister commented May 22, 2020

I am hyped to try it out, but gotta finish a git video tutorial first :/. Maybe I will play around. In that repo I linked I've been playing with type hinting totally outside of Python trying to bridge two worlds together :). Might be that little experience I do have can be helpful here.

You know what? I will try to do something here
If there's something to show, that's useful, I will let you know ;-)

@Delgan
Copy link
Owner

Delgan commented May 22, 2020

Great, thanks! But if it's too complicated, don't worry. ;)

@kornicameister
Copy link
Contributor Author

it's rather simple introspection of particular function calls, so it should be actually quite simple ;-)

@Delgan
Copy link
Owner

Delgan commented Jun 7, 2020

So, I modified the way the LogRecord is filled with extra.

The main downside is that it's no longer possible to use Formatter("[%(some_extra_field)s] %(message)s") with standard logging handler, but I think it's certainly less of a problem than the "Attempt to overwrite" error was.

Closing thie issue consequently. Discussion about mypy plugins moved here: #277

@Delgan Delgan closed this as completed Jun 7, 2020
jhamon pushed a commit to pinecone-io/pinecone-python-client that referenced this issue Oct 11, 2023
Bumps [loguru](https://github.com/Delgan/loguru) from 0.5.0 to 0.5.3.
<details>
<summary>Release notes</summary>
<p><em>Sourced from <a
href="https://github.com/Delgan/loguru/releases">loguru's
releases</a>.</em></p>
<blockquote>
<h2>0.5.3</h2>
<ul>
<li>Fix child process possibly hanging at exit while combining
<code>enqueue=True</code> with third party library like
<code>uwsgi</code> (<a
href="https://github.com/Delgan/loguru/issues/309">#309</a>,
thanks <a
href="https://github.com/dstlmrk"><code>@​dstlmrk</code></a>).</li>
<li>Fix possible exception during formatting of non-string messages (<a
href="https://github.com/Delgan/loguru/issues/331">#331</a>).</li>
</ul>
<h2>0.5.2</h2>
<ul>
<li>Fix <code>AttributeError</code> within handlers using
<code>serialize=True</code> when calling <code>logger.exception()</code>
outside of the context of an exception (<a
href="https://github.com/Delgan/loguru/issues/296">#296</a>).</li>
<li>Fix error while logging an exception containing a non-picklable
<code>value</code> to a handler with <code>enqueue=True</code> (<a
href="https://github.com/Delgan/loguru/issues/298">#298</a>).</li>
<li>Add support for async callable classes (with <code>__call__</code>
method) used as sinks (<a
href="https://github.com/Delgan/loguru/issues/294">#294</a>,
thanks <a
href="https://github.com/jessekrubin"><code>@​jessekrubin</code></a>).</li>
</ul>
<h2>0.5.1</h2>
<ul>
<li>Modify the way the <code>extra</code> dict is used by
<code>LogRecord</code> in order to prevent possible
<code>KeyError</code> with standard <code>logging</code> handlers (<a
href="https://github.com/Delgan/loguru/issues/271">#271</a>).</li>
<li>Add a new <code>default</code> optional argument to
<code>logger.catch()</code>, it should be the returned value by the
decorated function in case an error occurred (<a
href="https://github.com/Delgan/loguru/issues/272">#272</a>).</li>
<li>Fix <code>ValueError</code> when using <code>serialize=True</code>
in combination with <code>logger.catch()</code> or
<code>logger.opt(record=True)</code> due to circular reference of the
<code>record</code> dict (<a
href="https://github.com/Delgan/loguru/issues/286">#286</a>).</li>
</ul>
</blockquote>
</details>
<details>
<summary>Changelog</summary>
<p><em>Sourced from <a
href="https://github.com/Delgan/loguru/blob/master/CHANGELOG.rst">loguru's
changelog</a>.</em></p>
<blockquote>
<h1><code>0.5.3</code>_ (2020-09-20)</h1>
<ul>
<li>Fix child process possibly hanging at exit while combining
<code>enqueue=True</code> with third party library like
<code>uwsgi</code>
(<code>[#309](Delgan/loguru#309)
&lt;https://github.com/Delgan/loguru/issues/309&gt;</code><em>, thanks
<code>@dstlmrk &lt;https://github.com/dstlmrk&gt;</code></em>).</li>
<li>Fix possible exception during formatting of non-string messages
(<code>[#331](Delgan/loguru#331)
&lt;https://github.com/Delgan/loguru/issues/331&gt;</code>_).</li>
</ul>
<h1><code>0.5.2</code>_ (2020-09-06)</h1>
<ul>
<li>Fix <code>AttributeError</code> within handlers using
<code>serialize=True</code> when calling <code>logger.exception()</code>
outside of the context of an exception
(<code>[#296](Delgan/loguru#296)
&lt;https://github.com/Delgan/loguru/issues/296&gt;</code>_).</li>
<li>Fix error while logging an exception containing a non-picklable
<code>value</code> to a handler with <code>enqueue=True</code>
(<code>[#298](Delgan/loguru#298)
&lt;https://github.com/Delgan/loguru/issues/298&gt;</code>_).</li>
<li>Add support for async callable classes (with <code>__call__</code>
method) used as sinks
(<code>[#294](Delgan/loguru#294)
&lt;https://github.com/Delgan/loguru/pull/294&gt;</code><em>, thanks
<code>@jessekrubin
&lt;https://github.com/jessekrubin&gt;</code></em>).</li>
</ul>
<h1><code>0.5.1</code>_ (2020-06-12)</h1>
<ul>
<li>Modify the way the <code>extra</code> dict is used by
<code>LogRecord</code> in order to prevent possible
<code>KeyError</code> with standard <code>logging</code> handlers
(<code>[#271](Delgan/loguru#271)
&lt;https://github.com/Delgan/loguru/issues/271&gt;</code>_).</li>
<li>Add a new <code>default</code> optional argument to
<code>logger.catch()</code>, it should be the returned value by the
decorated function in case an error occurred
(<code>[#272](Delgan/loguru#272)
&lt;https://github.com/Delgan/loguru/issues/272&gt;</code>_).</li>
<li>Fix <code>ValueError</code> when using <code>serialize=True</code>
in combination with <code>logger.catch()</code> or
<code>logger.opt(record=True)</code> due to circular reference of the
<code>record</code> dict
(<code>[#286](Delgan/loguru#286)
&lt;https://github.com/Delgan/loguru/issues/286&gt;</code>_).</li>
</ul>
</blockquote>
</details>
<details>
<summary>Commits</summary>
<ul>
<li><a
href="https://github.com/Delgan/loguru/commit/f31e97142adc1156693a26ecaf47208d3765a6e3"><code>f31e971</code></a>
Bump version to 0.5.3</li>
<li><a
href="https://github.com/Delgan/loguru/commit/3394fdbd04efa7f5ec9c9625ec4ec34ca72b16e7"><code>3394fdb</code></a>
Update Changelog to reference <a
href="https://github.com/Delgan/loguru/issues/309">#309</a>
fix</li>
<li><a
href="https://github.com/Delgan/loguru/commit/44f677172ccf10ec60cbc46e4ac77394429211a7"><code>44f6771</code></a>
Fix removing simple queue in child processes (<a
href="https://github.com/Delgan/loguru/issues/325">#325</a>)</li>
<li><a
href="https://github.com/Delgan/loguru/commit/296635c33cbc735c3367728f24f07545dbfe19c5"><code>296635c</code></a>
Fix possible exception while formatting non-string message (<a
href="https://github.com/Delgan/loguru/issues/331">#331</a>)</li>
<li><a
href="https://github.com/Delgan/loguru/commit/b77f4fd23ac380fa2521dd2039c961af1218d1d1"><code>b77f4fd</code></a>
Correct simple typo in docs, propogates -&gt; propagates (<a
href="https://github.com/Delgan/loguru/issues/323">#323</a>)</li>
<li><a
href="https://github.com/Delgan/loguru/commit/e48f3e49228ada2f6495dd7eedc4ecd03530a1f6"><code>e48f3e4</code></a>
Bump version to 0.5.2</li>
<li><a
href="https://github.com/Delgan/loguru/commit/d93c19f6c1c53cc14e9afcb28b770f8aa43c28cd"><code>d93c19f</code></a>
Flesh out the few remaining incomplete annotations (<a
href="https://github.com/Delgan/loguru/issues/316">#316</a>)</li>
<li><a
href="https://github.com/Delgan/loguru/commit/084dffd16211e8494419ed5cf9c6859de8e76123"><code>084dffd</code></a>
Fix tests fails (since pytest upgrade) due to standard logging
misuse</li>
<li><a
href="https://github.com/Delgan/loguru/commit/9a40e58c74e786149b6a4c6097e24d50e0b40459"><code>9a40e58</code></a>
Fix error with &quot;enqueue=True&quot; and non-picklable exception (<a
href="https://github.com/Delgan/loguru/issues/298">#298</a>)</li>
<li><a
href="https://github.com/Delgan/loguru/commit/6f86f487590fe8f264a3bb26c722bf4be354687b"><code>6f86f48</code></a>
Refactor usage of &quot;inspect&quot; functions for simplification</li>
<li>Additional commits viewable in <a
href="https://github.com/Delgan/loguru/compare/0.5.0...0.5.3">compare
view</a></li>
</ul>
</details>
<br />


[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=loguru&package-manager=pip&previous-version=0.5.0&new-version=0.5.3)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)
You can disable automated security fix PRs for this repo from the
[Security Alerts
page](https://github.com/pinecone-io/pinecone-python-client/network/alerts).

</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to an already existing feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants