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

[Issue 6921][pulsar-broker-common] Replaced "Paths.get(...).getParent… #6992

Merged
merged 2 commits into from
Jun 4, 2020
Merged

[Issue 6921][pulsar-broker-common] Replaced "Paths.get(...).getParent… #6992

merged 2 commits into from
Jun 4, 2020

Conversation

horsteff
Copy link
Contributor

@horsteff horsteff commented May 19, 2020

…()", because it's system dependent and uses '\' as path separator on Windows

Fixes #6921

Motivation

Paths in Pulsar use '/' as separator char. In ConfigurationCacheService the method Path.getParent() is used, which is system dependent and uses '\' as path separator on Windows and so returns always null there.

Modifications

Replaced Path.getParent() with manually extracting the parent path

…()", because it's system dependent and uses '\' as path separator on Windows
@sijie
Copy link
Member

sijie commented May 26, 2020

/puslarbot run-failure-checks

@sijie sijie added this to the 2.6.0 milestone May 26, 2020
@sijie
Copy link
Member

sijie commented May 29, 2020

/puslarbot run-failure-checks

@horsteff
Copy link
Contributor Author

/pulsarbot run-failure-checks

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

This path is related to a ZooKeeper z-node. By using Path, on Windows it would translate into a \ which is incorrect for the z-node path.

@horsteff
Copy link
Contributor Author

This path is related to a ZooKeeper z-node. By using Path, on Windows it would translate into a \ which is incorrect for the z-node path.

Yes, that's the reason for this change - replacing the current use of class Path.

@sijie
Copy link
Member

sijie commented May 30, 2020

@merlimat I think your comment is the reason why @horsteff made this change. It removes the usage of Path.

@horsteff horsteff requested a review from merlimat June 1, 2020 11:30
@horsteff
Copy link
Contributor Author

horsteff commented Jun 2, 2020

/pulsarbot run-failure-checks

@merlimat
Copy link
Contributor

merlimat commented Jun 2, 2020

Got it. I got confused up.

@codelipenghui
Copy link
Contributor

/pulsarbot run-failure-checks

@merlimat merlimat merged commit da0492b into apache:master Jun 4, 2020
zeo1995 pushed a commit to zeo1995/pulsar that referenced this pull request Jun 5, 2020
…te-update

* 'website-update' of github.com:zeo1995/pulsar: (432 commits)
  Fixed ordering issue in KeyShared dispatcher when adding consumer (apache#7106)
  Fix Duplicated messages are sent to dead letter topic apache#6960 (apache#7021)
  [Issue 2793][Doc]--Update the TLS hostname verification for CPP and Python clients (apache#7162)
  [Doc]--set netty mex frame size (apache#7174)
  [Doc] Update for the maximum message size (apache#7171)
  Fixed KeyShared consumers getting stuck on delivery (apache#7105)
  [apache#6003][pulsar-functions] Possibility to add builtin Functions (apache#6895)
  [Issue 6921][pulsar-broker-common] Replaced "Paths.get(...).getParent()", because it's system dependent and uses '\' as path separator on Windows (apache#6992)
  Improve broker unit test CI (apache#7173)
  Fix typo in exception message (apache#7027)
  Support KeyValue Schema Use Null Key And Null Value (apache#7139)
  [Doc]--Update documents for support consumer priority level in failover mode (apache#7136)
  Add schema config to cpp and cgo docs. (apache#7137)
  [Doc]--Update for the maximum message size (apache#7160)
  [C++] Expose ZSTD and Snappy compression to C API (apache#7014)
  [pulsar-proxy] add proxyLogLevel into config file (apache#6948)
  Add multi-hosts example for bookkeeperMetadataServiceUri (apache#6998)
  support for termination of partitioned topic (apache#6126)
  Use pure-java Air-Compressor instead of JNI based libraries (apache#5390)
  [Issues 5709]remove the namespace checking (apache#5716)
  ...

# Conflicts:
#	site2/website/scripts/split-swagger-by-version.js
@horsteff horsteff deleted the remove-path-getparent branch July 8, 2020 20:16
cdbartholomew pushed a commit to kafkaesque-io/pulsar that referenced this pull request Jul 24, 2020
…()", because it's system dependent and uses '\' as path separator on Windows (apache#6992)

Co-authored-by: penghui <[email protected]>
huangdx0726 pushed a commit to huangdx0726/pulsar that referenced this pull request Aug 24, 2020
…()", because it's system dependent and uses '\' as path separator on Windows (apache#6992)

Co-authored-by: penghui <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IllegalArgumentException when starting PulsarStandalone
4 participants