Skip to content

[HOTFIX] Rename the identifier of driver from <driver> to driver#4245

Closed
sarutak wants to merge 1 commit into
apache:masterfrom
sarutak:hotfix-driver-identifier
Closed

[HOTFIX] Rename the identifier of driver from <driver> to driver#4245
sarutak wants to merge 1 commit into
apache:masterfrom
sarutak:hotfix-driver-identifier

Conversation

@sarutak

@sarutak sarutak commented Jan 28, 2015

Copy link
Copy Markdown
Member

This change is related to #3812.

The identifier of driver is defined as <driver> but this identifier is used for metrics name and some metrics sinks cannot parse < and >.

@pwendell

Copy link
Copy Markdown
Contributor

This could provide a usability regression for people since the name now appears differently in the UI. Is there any way we can intercept this somewhere downstream and not change it here?

@sarutak

sarutak commented Jan 28, 2015

Copy link
Copy Markdown
Member Author

How about escaping specific characters like < or > in each sink instead of changing the identifier?

@sarutak

sarutak commented Jan 28, 2015

Copy link
Copy Markdown
Member Author

I found this issue is resolved by updating com.codahale.metrics library from 3.0.0 to 3.1.0 and it's proposed in #4209. So I'll close this PR.

@sarutak sarutak closed this Jan 28, 2015
@SparkQA

SparkQA commented Jan 28, 2015

Copy link
Copy Markdown

Test build #26221 has finished for PR 4245 at commit af59bf2.

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

@tsingfu

tsingfu commented Mar 2, 2015

Copy link
Copy Markdown

@sarutak I found #4209 has not solved the problem invalid metric name with "" which introduced from #3812.
and I have a question: Is there difference between setting DRIVER_IDENTIFIER to "driver" and ""?

@sarutak sarutak deleted the hotfix-driver-identifier branch April 11, 2015 05:24
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