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

Reinstate statically computed log event ids #29878

Merged
merged 3 commits into from
Nov 1, 2018
Merged

Conversation

c42f
Copy link
Member

@c42f c42f commented Nov 1, 2018

Log ids are meant to identify the location of the message in the source code and must be computed at compile time.

For further commentary on this proposed fix see #29355 (comment) and #29355 (comment)

Fixes #28786, fixes #28400 and closes #29355.

@adamslc I incorporated your tests in here (lightly modified)

@oxinabox @iamed2 @samoconnor you'll need this to fix #28400 before using the new logging infrastructure in any serious way for long running tasks. [Yikes, what a horrible bug :-( ]

Luke Adams and others added 3 commits November 1, 2018 09:01
Log ids are meant to identify the location of the message in the source
code and must be computed at compile time.

Fixes #28786, #28400; replaces #29355.
@c42f c42f added logging The logging framework backport pending 1.0 labels Nov 1, 2018
@ararslan ararslan added the bugfix This change fixes an existing bug label Nov 1, 2018
@c42f c42f added the embarrassing-bugfix Whoops! label Nov 1, 2018
@c42f
Copy link
Member Author

c42f commented Nov 1, 2018

There was a couple of win32 CI profile test failures which looked bizarre and unrelated so I restarted that. Here's a separate issue for that: #29880

@adamslc adamslc mentioned this pull request Nov 1, 2018
@KristofferC KristofferC added the priority This should be addressed urgently label Nov 1, 2018
@KristofferC
Copy link
Sponsor Member

Putting a priority label on this since we want this in 1.0.2 which we want to run PkgEval and release as soon as possible.

@JeffBezanson
Copy link
Sponsor Member

LGTM.

@fredrikekre fredrikekre merged commit 51683c4 into master Nov 1, 2018
@fredrikekre fredrikekre deleted the cjf/fix-logging-ids branch November 1, 2018 18:15
KristofferC pushed a commit that referenced this pull request Nov 1, 2018
* fix bug and add tests

* Reinstate statically computed log record ids

Log ids are meant to identify the location of the message in the source
code and must be computed at compile time.

fix #28786, fix #28400; closes #29355.

* Clarify documentation regarding log record `id`

(cherry picked from commit 51683c4)
@c42f
Copy link
Member Author

c42f commented Nov 1, 2018

Thanks guys, getting this into 1.0.2 will be great.

@adamslc looks like your patch got mashed together with mine in the merge. Sorry about that :-/

@adamslc
Copy link
Contributor

adamslc commented Nov 1, 2018

No worries. Just glad to have this bug fixed.

KristofferC pushed a commit that referenced this pull request Nov 2, 2018
* fix bug and add tests

* Reinstate statically computed log record ids

Log ids are meant to identify the location of the message in the source
code and must be computed at compile time.

fix #28786, fix #28400; closes #29355.

* Clarify documentation regarding log record `id`

(cherry picked from commit 51683c4)
odow added a commit to jump-dev/JuMP.jl that referenced this pull request Dec 30, 2018
odow added a commit to jump-dev/JuMP.jl that referenced this pull request Dec 30, 2018
* Remove outdated comment

We retain `copy_names = true` for explicitness.

* Remove outdated comment.

This was fixed by JuliaLang/julia#29878
KristofferC pushed a commit that referenced this pull request Feb 11, 2019
* fix bug and add tests

* Reinstate statically computed log record ids

Log ids are meant to identify the location of the message in the source
code and must be computed at compile time.

fix #28786, fix #28400; closes #29355.

* Clarify documentation regarding log record `id`

(cherry picked from commit 51683c4)
KristofferC pushed a commit that referenced this pull request Feb 20, 2020
* fix bug and add tests

* Reinstate statically computed log record ids

Log ids are meant to identify the location of the message in the source
code and must be computed at compile time.

fix #28786, fix #28400; closes #29355.

* Clarify documentation regarding log record `id`

(cherry picked from commit 51683c4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug embarrassing-bugfix Whoops! logging The logging framework priority This should be addressed urgently
Projects
None yet
6 participants