Skip to content

Conversation

@Delta456
Copy link
Contributor

@Delta456 Delta456 commented Feb 7, 2024

Fixes #650

Description

Fixes #650.

Uses RelativePath::new function for platform independent relative paths.

EDIT: Uses std::path::MAIN_SEPARATOR instead of hardcore / which handles slashes accordingly.


This PR has:

  • been tested to ensure log ingestion and log query works.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added documentation for new or modified features or behaviors.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@Delta456
Copy link
Contributor Author

Delta456 commented Feb 7, 2024

I have read the CLA Document and I hereby sign the CLA

nitisht added a commit to parseablehq/.github that referenced this pull request Feb 7, 2024
@Delta456
Copy link
Contributor Author

Delta456 commented Feb 7, 2024

recheck

@nitisht nitisht requested a review from Eshanatnight February 7, 2024 16:58
@Delta456
Copy link
Contributor Author

Delta456 commented Feb 7, 2024

PS: This is my first time writing Rust code so would love code review.

"{}",
self.root.join(RelativePath::new(prefix).as_str()).display()
))
object_store::path::Path::parse(format!("{}", self.root.join(prefix.as_str()).display()).trim_start_matches(std::path::MAIN_SEPARATOR),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I rechecked and std::path::MAIN_SEPARATOR seems like a better solution here.

object_store::path::Path::parse(
format!("{}", self.root.join(prefix.as_str()).display()).trim_start_matches('/'),
format!("{}", self.root.join(prefix.as_str()).display())
.trim_start_matches(std::path::MAIN_SEPARATOR),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using std::path::MAIN_SEPARATOR is a better solution as it handles slashes accordingly.

@nitisht
Copy link
Member

nitisht commented Feb 16, 2024

@Delta456 do you have windows machine / have you tested this ?

@Delta456
Copy link
Contributor Author

Delta456 commented Feb 16, 2024

@Delta456 do you have windows machine / have you tested this ?

Yes, I have tested this on my Windows machine.

image

@nitisht nitisht merged commit 68aa486 into parseablehq:main Feb 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 16, 2024
@Delta456 Delta456 deleted the fix_abs_url branch February 16, 2024 07:31
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.

Illegal character "\\" on windows while generating path for parquet files?

2 participants