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

Allow spaces in input file paths #9426

Merged
merged 1 commit into from
Sep 22, 2022
Merged

Allow spaces in input file paths #9426

merged 1 commit into from
Sep 22, 2022

Conversation

KKcorps
Copy link
Contributor

@KKcorps KKcorps commented Sep 19, 2022

Replacing spaces with %20 in segment utils helps out with this process.
Not using URLEncoder here since it also encodes the / and : characters
due to which uri.getScheme returns null

@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2022

Codecov Report

Merging #9426 (6e311e0) into master (61fc919) will increase coverage by 0.06%.
The diff coverage is 83.33%.

@@             Coverage Diff              @@
##             master    #9426      +/-   ##
============================================
+ Coverage     69.77%   69.83%   +0.06%     
- Complexity     5092     5097       +5     
============================================
  Files          1890     1890              
  Lines        100654   100954     +300     
  Branches      15327    15348      +21     
============================================
+ Hits          70231    70506     +275     
- Misses        25445    25466      +21     
- Partials       4978     4982       +4     
Flag Coverage Δ
integration1 26.03% <50.00%> (-0.05%) ⬇️
integration2 24.75% <83.33%> (-0.05%) ⬇️
unittests1 67.09% <50.00%> (+0.07%) ⬆️
unittests2 15.35% <0.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...mon/segment/generation/SegmentGenerationUtils.java 58.00% <83.33%> (+1.75%) ⬆️
.../java/org/apache/pinot/spi/utils/BooleanUtils.java 33.33% <0.00%> (-66.67%) ⬇️
...ator/transform/function/CastTransformFunction.java 57.00% <0.00%> (-19.92%) ⬇️
...nt/local/startree/v2/store/StarTreeDataSource.java 40.00% <0.00%> (-13.34%) ⬇️
...ntroller/helix/core/minion/TaskMetricsEmitter.java 76.74% <0.00%> (-9.31%) ⬇️
...requesthandler/MultiStageBrokerRequestHandler.java 59.15% <0.00%> (-7.90%) ⬇️
...core/startree/operator/StarTreeFilterOperator.java 85.91% <0.00%> (-7.75%) ⬇️
...ot/segment/local/startree/OffHeapStarTreeNode.java 65.90% <0.00%> (-6.82%) ⬇️
...tream/kafka20/server/KafkaDataServerStartable.java 72.91% <0.00%> (-6.25%) ⬇️
...r/validation/RealtimeSegmentValidationManager.java 71.62% <0.00%> (-5.41%) ⬇️
... and 88 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

}


public static void main(String[] args) throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

You may not need this code change in your PR.

@@ -606,4 +607,26 @@ public void close()
throws IOException {
super.close();
}

public static String sanitizeURIString(String uriStr) {
Copy link
Member

Choose a reason for hiding this comment

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

Same here. You may not need this code change.

@KKcorps KKcorps reopened this Sep 21, 2022
Copy link
Member

@jackjlli jackjlli left a comment

Choose a reason for hiding this comment

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

LGTM.

@KKcorps KKcorps merged commit 69722b0 into apache:master Sep 22, 2022
@KKcorps KKcorps deleted the uri_patch branch September 26, 2022 05:50
61yao pushed a commit to 61yao/pinot that referenced this pull request Oct 3, 2022
Co-authored-by: Kartik Khare <kharekartik@Kartiks-MacBook-Pro.local>
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.

3 participants