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

Thread and Mixed Platform Support #3612

Closed
wants to merge 41 commits into from
Closed

Conversation

mitsuhiko
Copy link
Member

@mitsuhiko mitsuhiko commented Jun 29, 2016

This is a work in progress pull request that implements mixed platforms on a frame by frame basis as well as support for handling multiple threads and stacktraces. Among other things it also adds the baseline support for the new flexible symbolication support which is in #3634.

New Thread Rendering

Stacktrace only:

screen shot 2016-07-02 at 15 03 18

Exception:

screen shot 2016-07-02 at 15 03 07

## Dropdown

screen shot 2016-07-02 at 15 03 56

## Other things

The tag selector also uses lozenges now:

screen shot 2016-07-02 at 12 19 36

def to_python(cls, data):
threads = []

for thread in data.get('list') or ():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we call this values instead of list

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was considering it. I just want to discourage people to send this as a list.

@mitsuhiko mitsuhiko force-pushed the feature/new-crash-report branch 2 times, most recently from ceb069e to b73b7e6 Compare June 30, 2016 10:04
@mitsuhiko mitsuhiko mentioned this pull request Jul 1, 2016
9 tasks
@mitsuhiko mitsuhiko force-pushed the feature/new-crash-report branch 2 times, most recently from 4d22d7b to 680c296 Compare July 1, 2016 18:15
@@ -326,8 +326,6 @@ def normalize(self):
except Exception:
# XXX: we should consider logging this.
pass
else:
data['tags'].extend(inst.iter_tags())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assuming this was moved somewhere, but im not familiar with the code so just checking it was intentional

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this was accidentally left in which meant that tags in some circumstances were recorded twice.

@dcramer
Copy link
Member

dcramer commented Jul 1, 2016

I think we should improve some visuals here, but we could do that after merging.

I am a bit concerned about the overall event size. Do we know the rough size of a real-world payload with threads?

@mitsuhiko
Copy link
Member Author

I refactored the thread and exception rendering so that we can implement what we have in the mockup. I did not fully implement the mockup yet because I don't have the right component available to do the thread selector as @ckj had it but I think this might be a reasonable start.

@mitsuhiko mitsuhiko changed the title WIP: New Crash Report Handling Thread and Mixed Platform Support Jul 3, 2016
@mitsuhiko
Copy link
Member Author

As far as I'm concerned this is ready for merging.

@dcramer
Copy link
Member

dcramer commented Jul 5, 2016

I think this is good, though I'm not sure the pill solution is the best for this (as its semi unstructured and variable width). I think it'd also be nice to keep the entire thread title available (rather than just the short title) outside of the select-open view. If the screenshots are up to date it also looks like there's a bit of a margin issue. I think we can deal with these when we implement the sidebar version.

@mitsuhiko
Copy link
Member Author

Closing this for #3634 which has all the things now.

@mitsuhiko mitsuhiko closed this Jul 6, 2016
@evanpurkhiser evanpurkhiser deleted the feature/new-crash-report branch August 21, 2017 20:20
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants