feat: modernize parser bindings and release workflow#6
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Modernizes the pybgpkit-parser Rust/PyO3 bindings and packaging/release workflow, while expanding the Python API (route-level parsing, projected tuple iteration, filter helpers) and adding benchmark/test scaffolding.
Changes:
- Upgraded core dependencies (bgpkit-parser 0.17.0, PyO3 0.28 with
abi3-py39) and expanded the Python API surface insrc/lib.rs. - Added Python API tests/benchmarks and Rust Criterion benchmarks to validate behavior and measure overhead.
- Reworked CI/release workflows to build ABI3 wheels + sdist via
maturin-actionand publish to PyPI / create GitHub Releases.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
UPDATE_DESIGN.md |
Documents the intended one-PR upgrade plan and design decisions. |
src/lib.rs |
Major API expansion (RouteParser/RouteElem, projected tuple iterators, Filter helpers, batch iteration) and PyO3 0.28 adjustments. |
Cargo.toml |
Dependency bumps + Criterion bench configuration. |
pyproject.toml |
Adds PEP 621 project metadata and dev extras. |
README.md |
Documents new Filter API, projected iteration, utilities, and RouteParser. |
CHANGELOG.md |
Adds 0.7.0 release notes draft. |
BUILD.md |
Updates build/publish instructions for the new CI-driven release flow. |
benches/parse_bench.rs |
Adds Criterion benchmarks for native elem/route iteration. |
tests/test_api.py |
Adds API tests (with opt-in network smoke tests). |
tests/benchmark.py |
Adds a manual Python-side benchmark script. |
AGENTS.md |
Adds contributor knowledge base / conventions. |
.github/workflows/rust.yaml |
Adds a Python API test job to CI. |
.github/workflows/release.yml |
New release pipeline: checks, wheel/sdist builds, PyPI upload, GH release. |
.gitignore |
Ignores Python bytecode / __pycache__. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+8
to
+12
| description = "Python binding for bgpkit-parser" | ||
| readme = "README.md" | ||
| requires-python = ">=3.9" | ||
| license = "MIT" | ||
| classifiers = [ |
Comment on lines
7
to
+10
| push: | ||
| tags: | ||
| - v[0-9]+.* | ||
| - "v*" | ||
| workflow_dispatch: |
Comment on lines
+582
to
+597
| unsafe impl Send for Parser {} | ||
| unsafe impl Sync for Parser {} | ||
| unsafe impl Send for BatchIterator {} | ||
| unsafe impl Sync for BatchIterator {} | ||
| unsafe impl Send for RouteParser {} | ||
| unsafe impl Sync for RouteParser {} | ||
| unsafe impl Send for RouteBatchIterator {} | ||
| unsafe impl Sync for RouteBatchIterator {} | ||
| unsafe impl Send for TupleIterator {} | ||
| unsafe impl Sync for TupleIterator {} | ||
| unsafe impl Send for TupleBatchIterator {} | ||
| unsafe impl Sync for TupleBatchIterator {} | ||
| unsafe impl Send for RouteTupleIterator {} | ||
| unsafe impl Sync for RouteTupleIterator {} | ||
| unsafe impl Send for RouteTupleBatchIterator {} | ||
| unsafe impl Sync for RouteTupleBatchIterator {} |
Comment on lines
+748
to
+756
| fn __next__(mut slf: PyRefMut<Self>, py: Python) -> PyResult<Option<Py<PyTuple>>> { | ||
| let fields = slf.fields.clone(); | ||
| let Some(elem_iter) = slf.elem_iter.as_mut() else { | ||
| return Ok(None); | ||
| }; | ||
| elem_iter | ||
| .next() | ||
| .map(|elem| elem_to_tuple(py, elem, &fields)) | ||
| .transpose() |
Comment on lines
+767
to
+786
| let fields = slf.fields.clone(); | ||
| let batch_size = slf.batch_size; | ||
| let Some(elem_iter) = slf.elem_iter.as_mut() else { | ||
| return Ok(None); | ||
| }; | ||
|
|
||
| let elems = py.detach(|| { | ||
| elem_iter | ||
| .by_ref() | ||
| .take(batch_size) | ||
| .collect::<Vec<BgpElem>>() | ||
| }); | ||
| if elems.is_empty() { | ||
| slf.elem_iter = None; | ||
| return Ok(None); | ||
| } | ||
|
|
||
| elems | ||
| .into_iter() | ||
| .map(|elem| elem_to_tuple(py, elem, &fields)) |
Comment on lines
+913
to
+921
| fn __next__(mut slf: PyRefMut<Self>, py: Python) -> PyResult<Option<Py<PyTuple>>> { | ||
| let fields = slf.fields.clone(); | ||
| let Some(route_iter) = slf.route_iter.as_mut() else { | ||
| return Ok(None); | ||
| }; | ||
| route_iter | ||
| .next() | ||
| .map(|route| route_to_tuple(py, route, &fields)) | ||
| .transpose() |
Comment on lines
+932
to
+953
| let fields = slf.fields.clone(); | ||
| let batch_size = slf.batch_size; | ||
| let Some(route_iter) = slf.route_iter.as_mut() else { | ||
| return Ok(None); | ||
| }; | ||
|
|
||
| let routes = py.detach(|| { | ||
| route_iter | ||
| .by_ref() | ||
| .take(batch_size) | ||
| .collect::<Vec<BgpRouteElem>>() | ||
| }); | ||
| if routes.is_empty() { | ||
| slf.route_iter = None; | ||
| return Ok(None); | ||
| } | ||
|
|
||
| routes | ||
| .into_iter() | ||
| .map(|route| route_to_tuple(py, route, &fields)) | ||
| .collect::<PyResult<Vec<_>>>() | ||
| .map(Some) |
Comment on lines
+22
to
+34
| python-api: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.11" | ||
| - name: Install test tools | ||
| run: python -m pip install --upgrade pip maturin pytest pytest-benchmark | ||
| - name: Build extension in-place | ||
| run: maturin develop | ||
| - name: Run Python API tests | ||
| run: pytest tests/test_api.py |
Comment on lines
+80
to
+83
| - `bgpkit-parser` crate version bump is the primary release trigger (see CHANGELOG for version history) | ||
| - Release workflow: `rust.yaml` runs Rust + Python API checks on PR/push; `release.yml` builds ABI3 wheels and publishes on `v*` tag push | ||
| - Supports Python 3.9+ via ABI3 wheels | ||
| - No Python tests in-repo; examples in `examples/` serve as smoke tests |
Comment on lines
+370
to
+391
| #[staticmethod] | ||
| pub fn get_psv_header() -> String { | ||
| [ | ||
| "type", | ||
| "timestamp", | ||
| "peer_ip", | ||
| "peer_asn", | ||
| "prefix", | ||
| "as_path", | ||
| "origin_asns", | ||
| "origin", | ||
| "next_hop", | ||
| "local_pref", | ||
| "med", | ||
| "communities", | ||
| "atomic", | ||
| "aggr_asn", | ||
| "aggr_ip", | ||
| "only_to_customer", | ||
| ] | ||
| .join("|") | ||
| } |
Comment on lines
+393
to
+412
| pub fn to_psv(&self) -> String { | ||
| format!( | ||
| "{}|{}|{}|{}|{}|{}|{}|{}|{}|{}|{}|{}|{}|{}|{}|{}", | ||
| self.elem_type, | ||
| self.timestamp, | ||
| self.peer_ip, | ||
| self.peer_asn, | ||
| self.prefix, | ||
| option_to_string(&self.as_path), | ||
| option_vec_to_string(&self.origin_asns), | ||
| option_to_string(&self.origin), | ||
| option_to_string(&self.next_hop), | ||
| option_to_string(&self.local_pref), | ||
| option_to_string(&self.med), | ||
| option_vec_to_string(&self.communities), | ||
| option_to_string(&self.atomic), | ||
| option_to_string(&self.aggr_asn), | ||
| option_to_string(&self.aggr_ip), | ||
| option_to_string(&self.only_to_customer), | ||
| ) |
Comment on lines
+35
to
+38
| - name: Run Python API tests | ||
| run: pytest tests/test_api.py | ||
| env: | ||
| PYBGPKIT_RUN_NETWORK_TESTS: "1" |
Comment on lines
8
to
+11
| push: | ||
| tags: | ||
| - v[0-9]+.* | ||
| - "v[0-9]*.[0-9]*.[0-9]*" | ||
| workflow_dispatch: |
Comment on lines
+3
to
+6
| ## OVERVIEW | ||
|
|
||
| Python binding for `bgpkit-parser` (Rust MRT/BGP parser). Exposes a single `Parser` class and `Elem` dataclass via PyO3, built with maturin. | ||
|
|
Comment on lines
+39
to
+43
| - Rust fmt/clippy enforced in CI (`cargo fmt --check`, `cargo clippy -- -D warnings`) | ||
| - `PyValueError` used for filter errors propagated to Python | ||
| - `unsafe impl Send + Sync for Parser` — required because `ElemIterator<Box<dyn Send + Read>>` is not auto-Send | ||
| - `#[pyo3(name = "__str__")]` used for JSON string representation of `Elem` | ||
| - `atomic` field returns `"AG"`/`"NAG"` strings (not bool) |
Comment on lines
+79
to
+84
| ### 9. `unsafe impl Send/Sync` for `Parser` | ||
|
|
||
| **Decision:** Keep and verify after the `bgpkit-parser` bump. | ||
|
|
||
| **Rationale:** The `ElemIterator` type may have changed in v0.17.0. If `BgpkitParser::into_iter()` no longer returns `Send`, we switch to a different approach (e.g., `into_elem_iter()` or `into_fallible_elem_iter()`). | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Notes