Skip to content

Conversation

@lw-lin
Copy link
Contributor

@lw-lin lw-lin commented Mar 29, 2016

What changes were proposed in this pull request?

Currently the Streaming tab in web UI uses records and events interchangeably; this PR tries to standardize them on "records".

"records" is chosen over "events" because:

  • "records" is used extensively throughout the streaming documents, codes, and comments
  • "events" is used only in Streaming UI related codes and comments

How was this patch tested?

  • existing test suites
  • manually checking on the Streaming UI tab

@lw-lin
Copy link
Contributor Author

lw-lin commented Mar 29, 2016

No public documents need to change, since we have been using the term "records" consistently.

Strings on the Streaming tab has been changed from "events" to "records".

There will be very minimum change if we also clean up the codes a little too -- only one private class, one public method in another private class, and some local variable require renaming -- so I've also done it along within this PR.

@srowen @zsxwing would you mind taking a look when you have time? Thanks!

@SparkQA
Copy link

SparkQA commented Mar 29, 2016

Test build #54429 has finished for PR 12032 at commit 43e3032.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@lw-lin lw-lin changed the title [SPARK-12857][STREAMING] Streaming tab in web UI uses records and events interchangeably [SPARK-12857][STREAMING] standardize "records" and "events" on "records" Mar 29, 2016
@lw-lin lw-lin changed the title [SPARK-12857][STREAMING] standardize "records" and "events" on "records" [SPARK-12857][STREAMING] Standardize "records" and "events" on "records" Mar 29, 2016
"all-stream-events-timeline",
"all-stream-events-histogram",
eventRateForAllStreams.data,
"all-stream-records-timeline",
Copy link
Member

Choose a reason for hiding this comment

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

The other changes seem OK as they're to labels and private classes and internal code; this changes a div ID and I'm not clear if something else has to change in the code to match? like will something else fail because it doesn't find all-stream-events-timeline? I don't see any other references though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@srowen thank you for the detailed review!

Yeah normally this requires something else to change, but GraphUIData has been implemented well enough to save the trouble. We only need to change this "all-stream-events-timeline" here, then the div ID as well as any other place referring to this div will change accordingly; please see the snapshots of the HTML source:

div as a placeholder:
records-2-html-1

then fill in that div with data:
records-2-html-2

Thanks!

@lw-lin
Copy link
Contributor Author

lw-lin commented Mar 30, 2016

@zsxwing please be aware of the changes we made here to address the problem reported in [Spark-12857] by Jacek Laskowski.

:-)

@zsxwing
Copy link
Member

zsxwing commented Mar 31, 2016

LGTM

@srowen
Copy link
Member

srowen commented Apr 1, 2016

Merged to master

@asfgit asfgit closed this in 19f32f2 Apr 1, 2016
@lw-lin lw-lin deleted the streaming-events-to-records branch June 5, 2016 02:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants