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

Analytics Plugin doesn't works for GoCD windows instance #4923

Closed
GaneshSPatil opened this issue Jul 6, 2018 · 2 comments

Comments

Projects
None yet
3 participants
@GaneshSPatil
Copy link
Contributor

commented Jul 6, 2018

Issue Type
  • Bug Report
Summary

Windows GoCD server fails to load any analytics chart to the path issue.
This issue is while creating an URI from a file path. The system-dependent default name-separator character needs to be used instead of hardcoding it to /.
On UNIX systems /, whereas, on windows systems \\ is used as name-separator character.

Go Server Log:

2018-07-05 15:55:48,037 ERROR [qtp1697511312-49] AnalyticsDelegate:106 - Encountered error while fetching analytics 
java.lang.IllegalArgumentException: Illegal character in path at index 6: assets\plugins\com.thoughtworks.gocd.analytics\B31EA0A3366304AFA5C012D7250339F125C132BC6DA9599541318B40982EEB90/agent-with-highest-utilization-chart.html
Basic environment details
  • Go Version: v18.6.0
  • Plugin Version: 1.0.0-683
Expected Results

GoCD Server should load all the analytics charts on windows operating systems.

Actual Results

GoCD Server fails to load all the analytics charts on windows operating systems.

Possible Fix

Replace / with the operating specific name sepearator character.
Use File.separator which is returns system-dependent default name-separator character.

@GaneshSPatil GaneshSPatil added this to the Release 18.7 milestone Jul 6, 2018

@GaneshSPatil GaneshSPatil self-assigned this Jul 6, 2018

GaneshSPatil added a commit to GaneshSPatil/gocd that referenced this issue Jul 6, 2018

Use File.separator instead of '/' for generating the URI while fetchi…
…ng the FullViewPath of a given analytics (gocd#4923)

* The URI path creation fails on windows due to '/' being used as a name seperator character.
* Use File.separator which returns system-dependent default name-separator character. On UNIX systems '/', whereas, on windows systems '\\' is used as name-separator character.
@marques-work

This comment has been minimized.

Copy link
Member

commented Jul 8, 2018

I actually think we need to enforce forward slashes /, as this is going to be a URL that the browser will load. The exception is complaining that the \ is not a valid character to pass to URI.create().

Illegal character in path at index 6: assets\plugins\com.thoughtworks.gocd.analytics\B31EA0A3366304AFA5C012D7250339F125C132BC6DA9599541318B40982EEB90/agent-with-highest-utilization-chart.html

Character at index 6 (i.e. the 7th character) is \.

Rather, we should do this conceptually: URI.join(assetRoot.gsub(File.separator, "/"), fileName). Of course that snippet isn't actually Java, so it won't work, but you get the idea :).

See com.thoughtworks.go.plugin.domain.analytics.AnalyticsData#getFullViewPath()

GaneshSPatil added a commit to GaneshSPatil/gocd that referenced this issue Jul 9, 2018

Use FilenameUtils.separatorsToUnix to convert all the '\\' to '/' whi…
…ch is a valid name seperator character in the URI (gocd#4923)

* The URI path creation fails on windows due to unsupported character '\\' being used in the URI.
* Convert all the name seperator character to the Unix name seperator '/' before creating the URI from the file path.

GaneshSPatil added a commit to GaneshSPatil/gocd that referenced this issue Jul 9, 2018

Use FilenameUtils.separatorsToUnix to convert all the '\\' to '/' whi…
…ch is a valid name seperator character in the URI (gocd#4923)

* The URI path creation fails on windows due to unsupported character '\\' being used in the URI.
* Convert all the name seperator character to the Unix name seperator '/' before creating the URI from the file path.

maheshp added a commit that referenced this issue Jul 9, 2018

Merge pull request #4924 from GaneshSPatil/os-dependent-name-seperato…
…r-analytics

    Use FilenameUtils.separatorsToUnix to convert all the '\\' to '/' which is a valid name seperator character in the URI (#4923)
@rajiesh

This comment has been minimized.

Copy link
Contributor

commented Jul 18, 2018

Verified on 18.7.0 (7107-ca37138fb0de89afb2f607cb57de22478550f8ac

@rajiesh rajiesh closed this Jul 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.