You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "Yicong-Huang (via GitHub)" <gi...@apache.org> on 2026/05/02 07:47:43 UTC

[PR] test(amber-python): cover sendsemantics partitioners [texera]

Yicong-Huang opened a new pull request, #4664:
URL: https://github.com/apache/texera/pull/4664

   ### What changes were proposed in this PR?
   
   Add a single test module `core/architecture/sendsemantics/test_partitioners.py` that exercises each concrete partitioner under that package:
   
   - `BroadcastPartitioner`
   - `RoundRobinPartitioner`
   - `OneToOnePartitioner`
   - `HashBasedShufflePartitioner`
   - `RangeBasedShufflePartitioner`
   
   For each one the suite covers:
   
   - constructor — receivers list, batch size, partitioner-specific attributes (hash keys, range bounds, worker_id matching, deduped channels)
   - `add_tuple_to_batch` — under-batch (no yield), at-batch (yield + slot reset), routing correctness
   - `flush` — pending-batch + ECM emitted only to the target receiver, empty-batch case, unknown-receiver case
   - `flush_state` — pending batches plus a per-receiver state record
   - `reset` — pending state cleared
   - `RangeBasedShufflePartitioner.get_receiver_index` — below `range_min`, above `range_max`, in-range quotient, boundary values
   
   Helper `_snapshot()` captures yielded list payloads at yield time, since several partitioners yield a receiver's pending batch by reference and clear it on the next statement of the generator.
   
   **Coverage on `core/architecture/sendsemantics/` rises from 56.6% to 99%.** All 5 concrete partitioner files reach 100%; the only remaining uncovered lines are the abstract-method `pass` stubs in the `Partitioner` base class.
   
   ```
   Name                                                                 Stmts   Miss  Cover
   core/architecture/sendsemantics/__init__.py                              0      0   100%
   core/architecture/sendsemantics/broad_cast_partitioner.py               44      0   100%
   core/architecture/sendsemantics/hash_based_shuffle_partitioner.py       40      0   100%
   core/architecture/sendsemantics/one_to_one_partitioner.py               40      0   100%
   core/architecture/sendsemantics/partitioner.py                          23      5    78%   (abstract pass-stubs)
   core/architecture/sendsemantics/range_based_shuffle_partitioner.py      49      0   100%
   core/architecture/sendsemantics/round_robin_partitioner.py              39      0   100%
   TOTAL                                                                  474      5    99%
   ```
   
   ### Any related issues, documentation, discussions?
   
   Closes #4663.
   
   ### How was this PR tested?
   
   \`\`\`
   $ python3 -m pytest core/architecture/sendsemantics/test_partitioners.py -v
   ... 35 passed in 0.65s
   $ python3 -m coverage run --source=core/architecture/sendsemantics -m pytest core/architecture/sendsemantics/test_partitioners.py
   $ python3 -m coverage report -m
   ... TOTAL 474 5 99%
   \`\`\`
   
   The new tests run under the existing python CI job (no new dependencies, no new fixtures needing infra). Codecov should pick the bump up automatically once the python job uploads coverage on this PR.
   
   ### Was this PR authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Opus 4.7, 1M context)


-- 
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: notifications-unsubscribe@texera.apache.org

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


Re: [PR] test(amber-python): cover sendsemantics partitioners [texera]

Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #4664:
URL: https://github.com/apache/texera/pull/4664#discussion_r3176358215


##########
amber/src/main/python/core/architecture/sendsemantics/test_partitioners.py:
##########
@@ -0,0 +1,434 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import pytest
+
+from core.architecture.sendsemantics.broad_cast_partitioner import BroadcastPartitioner
+from core.architecture.sendsemantics.hash_based_shuffle_partitioner import (
+    HashBasedShufflePartitioner,
+)
+from core.architecture.sendsemantics.one_to_one_partitioner import OneToOnePartitioner
+from core.architecture.sendsemantics.range_based_shuffle_partitioner import (
+    RangeBasedShufflePartitioner,
+)
+from core.architecture.sendsemantics.round_robin_partitioner import (
+    RoundRobinPartitioner,
+)
+from core.models import Tuple
+from core.models.schema.schema import Schema
+from core.models.state import State
+from proto.org.apache.texera.amber.core import (
+    ActorVirtualIdentity,
+    ChannelIdentity,
+)
+from proto.org.apache.texera.amber.engine.architecture.rpc import (
+    EmbeddedControlMessage,
+)
+from proto.org.apache.texera.amber.engine.architecture.sendsemantics import (
+    BroadcastPartitioning,

Review Comment:
   Are they all being used? Especially the BroadcastPartitioning



-- 
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: notifications-unsubscribe@texera.apache.org

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


Re: [PR] test(amber-python): cover sendsemantics partitioners [texera]

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

   ## [Codecov](https://app.codecov.io/gh/apache/texera/pull/4664?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.
   :warning: Please [upload](https://docs.codecov.com/docs/codecov-uploader) report for BASE (`main@69f3aea`). [Learn more](https://docs.codecov.io/docs/error-reference?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#section-missing-base-commit) about missing BASE report.
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@           Coverage Diff           @@
   ##             main    #4664   +/-   ##
   =======================================
     Coverage        ?   85.05%           
   =======================================
     Files           ?      157           
     Lines           ?     7803           
     Branches        ?        0           
   =======================================
     Hits            ?     6637           
     Misses          ?     1166           
     Partials        ?        0           
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/texera/pull/4664/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [python](https://app.codecov.io/gh/apache/texera/pull/4664/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.05% <100.00%> (?)` | |
   
   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 Sentry](https://app.codecov.io/gh/apache/texera/pull/4664?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>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :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: notifications-unsubscribe@texera.apache.org

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


Re: [PR] test(amber-python): cover sendsemantics partitioners [texera]

Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #4664:
URL: https://github.com/apache/texera/pull/4664#discussion_r3176373130


##########
amber/src/main/python/core/architecture/sendsemantics/test_partitioners.py:
##########
@@ -0,0 +1,434 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import pytest
+
+from core.architecture.sendsemantics.broad_cast_partitioner import BroadcastPartitioner
+from core.architecture.sendsemantics.hash_based_shuffle_partitioner import (
+    HashBasedShufflePartitioner,
+)
+from core.architecture.sendsemantics.one_to_one_partitioner import OneToOnePartitioner
+from core.architecture.sendsemantics.range_based_shuffle_partitioner import (
+    RangeBasedShufflePartitioner,
+)
+from core.architecture.sendsemantics.round_robin_partitioner import (
+    RoundRobinPartitioner,
+)
+from core.models import Tuple
+from core.models.schema.schema import Schema
+from core.models.state import State
+from proto.org.apache.texera.amber.core import (
+    ActorVirtualIdentity,
+    ChannelIdentity,
+)
+from proto.org.apache.texera.amber.engine.architecture.rpc import (
+    EmbeddedControlMessage,
+)
+from proto.org.apache.texera.amber.engine.architecture.sendsemantics import (
+    BroadcastPartitioning,

Review Comment:
   yes, user can choose partitioning on the UI. it (at least was) very useful for sending multiple ML models to inference workers.



-- 
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: notifications-unsubscribe@texera.apache.org

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


Re: [PR] test(pyamber): cover sendsemantics partitioners [texera]

Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang merged PR #4664:
URL: https://github.com/apache/texera/pull/4664


-- 
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: notifications-unsubscribe@texera.apache.org

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