You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Vamsi-klu (via GitHub)" <gi...@apache.org> on 2026/07/21 04:27:48 UTC

[PR] docs(comments): align PathMatcher include/exclude comments with glob+regex behavior (#8192) [pinot]

Vamsi-klu opened a new pull request, #19025:
URL: https://github.com/apache/pinot/pull/19025

   ## Summary
   
   Aligns source comments and the ingestion job-spec YAML template with real PathMatcher behavior (glob **or** regex, whole path, URI normalization). **No runtime behavior change.**
   
   **Issues:** Related to apache/pinot#8192 / apache/pinot#10611 (docs PR lands in pinot-docs)
   
   ---
   
   ## What is the problem?
   
   Even after docs fixes, in-tree comments and the job-spec template still said “glob only” or showed fragile patterns. Contributors copy those comments back into docs and samples, reintroducing the same wrong guidance (#10611/#8192).
   
   ---
   
   ## Why did I do this?
   
   Comment/template drift is how incorrect path-matcher guidance keeps returning. A small comment-only PR keeps source and docs consistent without mixing behavioral changes.
   
   ---
   
   ## What is the implementation edit?
   
   | File | Change |
   |------|--------|
   | `pinot-spi/.../SegmentGenerationJobSpec.java` | Javadoc: PathMatcher, Java regex, normalization, templating |
   | `pinot-spi/.../ingestion_job_spec_template.yaml` | Matching comments / example patterns |
   | `pinot-common/.../SegmentGenerationUtils.java` | Param docs: glob or regex PathMatcher; typo `ture` → `true` |
   | `pinot-plugins/.../SegmentGenerationAndPushTaskGenerator.java` | Comment that include/exclude accept glob+regex |
   
   No matching logic changes.
   
   ---
   
   ## What is the impact?
   
   - **Contributors:** Less likely to reintroduce glob-only docs.
   - **Runtime:** None.
   - **Risk:** Minimal (comments/docs in source only).
   
   ---
   
   ## What is the testing edit?
   
   - No new tests (comment-only).
   - Spotless/license not strictly required for comments but PR should pass Apache checks.
   - Behavioral docs validated separately in pinot-docs PRs.
   
   ---
   
   ## Checklist
   
   - [x] Draft
   - [x] No behavior change
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] docs(comments): align PathMatcher include/exclude comments with glob+regex behavior (#8192) [pinot]

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19025:
URL: https://github.com/apache/pinot/pull/19025#issuecomment-5030537264

   ## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/19025?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 37.58%. Comparing base ([`b9e538a`](https://app.codecov.io/gh/apache/pinot/commit/b9e538a117e7a7934c6b656d7e0f77b6e89ee0b3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`a77cbe7`](https://app.codecov.io/gh/apache/pinot/commit/a77cbe705d4c3ef0ece69bcf338945b6eea49722?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   > :exclamation:  There is a different number of reports uploaded between BASE (b9e538a) and HEAD (a77cbe7). Click for more details.
   > 
   > <details><summary>HEAD has 9 uploads less than BASE</summary>
   >
   >| Flag | BASE (b9e538a) | HEAD (a77cbe7) |
   >|------|------|------|
   >|unittests1|1|0|
   >|java-25|4|2|
   >|temurin|4|2|
   >|integration|3|1|
   >|integration2|1|0|
   >|custom-integration1|1|0|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@              Coverage Diff              @@
   ##             master   #19025       +/-   ##
   =============================================
   - Coverage     57.10%   37.58%   -19.53%     
   - Complexity        7     1404     +1397     
   =============================================
     Files          2629     3423      +794     
     Lines        156315   215968    +59653     
     Branches      25533    34186     +8653     
   =============================================
   - Hits          89271    81172     -8099     
   - Misses        59423   127376    +67953     
   + Partials       7621     7420      -201     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/pinot/pull/19025/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/19025/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [integration](https://app.codecov.io/gh/apache/pinot/pull/19025/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
   | [integration1](https://app.codecov.io/gh/apache/pinot/pull/19025/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
   | [integration2](https://app.codecov.io/gh/apache/pinot/pull/19025/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [java-25](https://app.codecov.io/gh/apache/pinot/pull/19025/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.58% <ø> (-19.53%)` | :arrow_down: |
   | [temurin](https://app.codecov.io/gh/apache/pinot/pull/19025/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.58% <ø> (-19.53%)` | :arrow_down: |
   | [unittests](https://app.codecov.io/gh/apache/pinot/pull/19025/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.58% <ø> (-19.53%)` | :arrow_down: |
   | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/19025/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/19025/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.58% <ø> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/19025?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] docs(comments): align PathMatcher include/exclude comments with glob+regex behavior (#8192) [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #19025:
URL: https://github.com/apache/pinot/pull/19025


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] docs(comments): align PathMatcher include/exclude comments with glob+regex behavior (#8192) [pinot]

Posted by "Vamsi-klu (via GitHub)" <gi...@apache.org>.
Vamsi-klu commented on PR #19025:
URL: https://github.com/apache/pinot/pull/19025#issuecomment-5030174095

   Ready for review.
   
   Comment-only alignment of PathMatcher include/exclude docs in source (apache/pinot#8192). No runtime behavior change. Companion user docs are in pinot-contrib/pinot-docs#923.
   
   Please take a look when you have a chance:
   - @Jackie-Jiang
   - @xiangfu0
   
   Happy to tweak comment wording based on feedback.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] docs(comments): align PathMatcher include/exclude comments with glob+regex behavior (#8192) [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #19025:
URL: https://github.com/apache/pinot/pull/19025#issuecomment-5208728097

   Can you take a look at the test failure? Seems related


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] docs(comments): align PathMatcher include/exclude comments with glob+regex behavior (#8192) [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #19025:
URL: https://github.com/apache/pinot/pull/19025#issuecomment-5196531370

   @Vamsi-klu Can you please rebase to the latest master?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] docs(comments): align PathMatcher include/exclude comments with glob+regex behavior (#8192) [pinot]

Posted by "Vamsi-klu (via GitHub)" <gi...@apache.org>.
Vamsi-klu commented on PR #19025:
URL: https://github.com/apache/pinot/pull/19025#issuecomment-5249496835

   You were right that it was related. The reworded comments I added to the test template YAML contained raw dollar signs, and `GroovyTemplateUtilsTest` and `IngestionJobLauncherTest` render that file through Groovy's SimpleTemplateEngine, which cannot parse a bare dollar sign. Reworded to avoid dollar signs and backslashes entirely, rebased onto latest master, pushed as `adc0b5f`.
   
   I checked it by replicating `GroovyTemplateUtils.renderTemplate` against the same Groovy and SnakeYAML jars and rendering all five `IngestionJobLauncherTest` context variants: master renders 5 of 5, the old PR head failed, and the new one is back to 5 of 5 with the asserted values unchanged.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org