Repository navigation
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Module shadowing leaves eight migrated examples with invalid attribute lookups that prevent startup or Interest expression.
0 open findings
What changed in this PR
This breaking migration adopts dataclass-based TLV models across python-ndn and makes ndn.app the canonical application API.
Changes:
- Migrate protocol models, validators, tools, and examples.
- Remove legacy application and schema APIs.
- Add wire-format regression tests and update documentation.
| File | Description |
|---|---|
| tests/security/signer_test.py | Assert the new validator contract. |
| tests/misc/svs_tlv_test.py | Test SVS dataclass wire formats. |
| tests/misc/security_v2_test.py | Test certificates and SafeBag encoding. |
| tests/misc/nfd_mgmt_test.py | Test management command and response formats. |
| tests/misc/light_versec_test.py | Test binary model round trips. |
| tests/integration/app_v2_test.py | Remove the separate v2 integration suite. |
| tests/integration/app_test.py | Migrate application tests and cover registration. |
| tests/encoding/ndnlp_v2_test.py | Extend NDNLP encoding and parsing coverage. |
| tests/encoding/ndn_format_0_3_test.py | Add packet and signature regression tests. |
| tests/encoding/ndn_format_0_3_2017_test.py | Test legacy packet-format compatibility. |
| src/ndn/types.py | Remove legacy callback aliases. |
| src/ndn/security/validator/known_key_validator.py | Return ValidResult from known-key validators. |
| src/ndn/security/validator/digest_validator.py | Update digest validation; remove legacy composition. |
| src/ndn/security/validator/cascade_validator.py | Remove the cascade validator. |
| src/ndn/security/validator/__init__.py | Update validator exports. |
| src/ndn/schema/util.py | Remove schema pattern utilities. |
| src/ndn/schema/simple_trust.py | Remove schema trust policies. |
| src/ndn/schema/simple_node.py | Remove schema resource nodes. |
| src/ndn/schema/simple_cache.py | Remove schema caching. |
| src/ndn/schema/schema_tree.py | Remove the namespace schema framework. |
| src/ndn/schema/policy.py | Remove schema policy interfaces. |
| src/ndn/schema/__init__.py | Remove the schema package initializer. |
| src/ndn/name_tree.py | Remove legacy application tree helpers. |
| src/ndn/encoding/tlv_model_v2.py | Retire the separate v2 codec module. |
| src/ndn/encoding/ndnlp_v2.py | Convert NDNLP models to dataclasses. |
| src/ndn/bin/tools/cmd_serve_rdrcontent.py | Switch to canonical application imports. |
| src/ndn/bin/tools/cmd_serve_data.py | Switch to canonical application imports. |
| src/ndn/bin/tools/cmd_fetch_rdrcontent.py | Switch to canonical application imports. |
| src/ndn/bin/tools/cmd_fetch_data.py | Switch to canonical application imports. |
| src/ndn/bin/nfdc/utils.py | Rename the command-expression helper. |
| src/ndn/bin/nfdc/cmd_set_strategy.py | Update application and command-helper references. |
| src/ndn/bin/nfdc/cmd_remove_strategy.py | Update application and command-helper references. |
| src/ndn/bin/nfdc/cmd_remove_route.py | Update application and command-helper references. |
| src/ndn/bin/nfdc/cmd_remove_face.py | Use dataclass encoding for face management. |
| src/ndn/bin/nfdc/cmd_new_route.py | Update command expression. |
| src/ndn/bin/nfdc/cmd_new_face.py | Pass URI strings to dataclass models. |
| src/ndn/bin/nfdc/cmd_get_strategy.py | Parse strategy dataclasses. |
| src/ndn/bin/nfdc/cmd_get_status.py | Parse status dataclasses. |
| src/ndn/bin/nfdc/cmd_get_route.py | Parse routing dataclasses. |
| src/ndn/bin/nfdc/cmd_get_face.py | Encode queries and parse face dataclasses. |
| src/ndn/appv2.py | Remove the separate v2 application module. |
| src/ndn/app_support/svs/tlv.py | Convert SVS models to dataclasses. |
| src/ndn/app_support/svs/sync.py | Adopt canonical application and codec APIs. |
| src/ndn/app_support/segment_fetcher.py | Remove the legacy segment fetcher. |
| src/ndn/app_support/security_v2.py | Migrate certificate models and signing. |
| src/ndn/app_support/nfd_mgmt.py | Migrate management models and helpers. |
| src/ndn/app_support/light_versec/validator.py | Remove legacy LVS validation integration. |
| src/ndn/app_support/light_versec/binary.py | Convert LVS binary models to dataclasses. |
| src/ndn/app_support/light_versec/__init__.py | Remove legacy validator imports. |
| src/ndn/app_support/keychain_register.py | Use canonical application imports. |
| src/ndn/app_support/dispatcher.py | Remove the legacy dispatcher. |
| examples/rpc_producer.py | Remove the legacy RPC producer. |
| examples/rpc_consumer.py | Remove the legacy RPC consumer. |
| examples/rdrnode.py | Remove the schema-based RDR example. |
| examples/putchunks.py | Remove the legacy chunk producer. |
| examples/producer.py | Remove the legacy producer example. |
| examples/nfd_status.py | Remove the legacy status example. |
| examples/lvs/producer.py | Remove the legacy LVS producer. |
| examples/lvs/consumer.py | Remove the legacy LVS consumer. |
| examples/dpdk_experimental/udp_producer.py | Migrate DPDK producer application references. |
| examples/dpdk_experimental/udp_consumer.py | Migrate DPDK consumer application references. |
| examples/consumer.py | Remove the legacy consumer example. |
| examples/catchunks.py | Remove the legacy chunk consumer. |
| examples/app/svs/sync_example.py | Migrate SVS application references. |
| examples/app/keychain_cert/keychain_register.py | Use the canonical application constructor. |
| examples/app/keychain_cert/fetch_certificate.py | Migrate certificate-fetching application references. |
| examples/app/forwarding_hint/producer.py | Migrate producer application references. |
| examples/app/forwarding_hint/consumer.py | Migrate consumer application references. |
| examples/app/basic_packets/producer.py | Migrate basic producer application references. |
| examples/app/basic_packets/consumer.py | Migrate basic consumer application references. |
| docs/src/security/security.rst | Document the new validator contract. |
| docs/src/schema/utils.rst | Remove schema utility documentation. |
| docs/src/schema/schema.rst | Remove the schema overview. |
| docs/src/schema/schema_tree.rst | Remove schema tree documentation. |
| docs/src/schema/policies.rst | Remove schema policy documentation. |
| docs/src/schema/ex1.rst | Remove the schema file-sharing tutorial. |
| docs/src/schema/custom_node.rst | Remove custom-node documentation. |
| docs/src/lvs/package.rst | Update the type-alias documentation directive. |
| docs/src/examples/tlv_model.rst | Replace descriptor examples with dataclasses. |
| docs/src/examples/basic_app.rst | Document canonical consumer and producer APIs. |
| docs/src/encoding/tlv_model.rst | Document the dataclass codec API. |
| docs/src/encoding/encoding.rst | Update the encoding overview. |
| docs/src/appv2.rst | Remove the separate v2 application reference. |
| docs/src/app.rst | Document the canonical application API. |
| docs/index.rst | Remove retired documentation entries. |
| CHANGELOG.rst | Record the breaking migration and removals. |
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Co-authored-by: Cursor <cursoragent@cursor.com>
|
@yoursunny Do you mind giving a review of this new version? Thanks! |
| * Replace the descriptor-based TLV model with dataclass models and ``tlv_encode``/``tlv_parse``. | ||
| * Make the PIT-token-aware application API canonical at ``ndn.app`` and remove ``ndn.appv2``. | ||
| * Remove the legacy application API, Name Tree Schema, dispatcher, segment fetcher, and cascade validator. | ||
|
|
There was a problem hiding this comment.
v0.5.0 and v0.5.2 are missing.
|
|
||
|
|
||
| app = appv2.NDNApp() | ||
| app = app.NDNApp() |
There was a problem hiding this comment.
The bug below is flagged by Meta Muse, and "I" means Muse.
Consider this pattern instead for all examples and documentation:
from ndn.app import NDNApp, pass_all, ReplyFunc
app = NDNApp()The bug: module shadowing breaks all 8 migrated examples. The PR mechanically renamed appv2 to app in the examples, producing this pattern:
from ndn import app
app = app.NDNApp() # module name now shadowed by the instancepass_all, ReplyFunc, and PktContext are module-level names in ndn/app.py, not attributes of NDNApp. So after the rebinding:
- 5 files fail at runtime on
app.pass_all:basic_packets/consumer.py,forwarding_hint/consumer.py,keychain_cert/fetch_certificate.py,svs/sync_example.py,dpdk_experimental/udp_consumer.py - 3 files fail at import on
app.ReplyFunc/app.PktContextin handler annotations:basic_packets/producer.py,forwarding_hint/producer.py,dpdk_experimental/udp_producer.py
This is new in this PR — the old code did app = appv2.NDNApp(), which left the module name intact. Copilot flagged the same thing ("eight migrated examples"); I verified each file by hand, and the count matches exactly. Fix is trivial: don't name the instance app (or alias the import).
There was a problem hiding this comment.
This is the only example that demonstrates app_param usage at the consumer side.
Thus, it should be restored and migrated to new APIs.
Below justification is written by Meta Muse:
The deleted rpc_consumer.py was the only example that put ApplicationParameters into an expressed Interest. This matters more than it looks, because the new API changed the rules: express() raises ValueError if you pass app_param without a signer ("An Interest with AppParam is required to be signed") — while the old rpc_consumer.py passed app_param.encode() with no signer at all. So the old pattern doesn't port mechanically; there's now a signer requirement that no example demonstrates and that's easy to trip over.
There was a problem hiding this comment.
SegmentFetcher is a commonly used primitive in many applications.
It is not non-implementable in the new APIs.
Thus, please restore it.
There was a problem hiding this comment.
Light VecSec (LVS) is a flagship feature of this library.
The lvs_validator is an important part of LVS that is useful for applications.
Hence, please restore this validator along with its examples.
There was a problem hiding this comment.
This bug is flagged by Meta Muse:
Fetch-Data -a/--app-param is broken. express() raises ValueError when app_param is passed without a signer, the tool passes no signer, and its except clauses don't catch ValueError — so using -a crashes with an unhandled traceback out of run_forever. The old appv2.express had the same check, so this predates the PR, but it's now the only app_param sender left in the tree
There was a problem hiding this comment.
Nothing helps the developer write the correct metadata, because there is no type hints for the field(metadata=).
Meta Muse has suggested a custom alternative to field class, like this:
def tlv_field(tlv_type, *, default=..., default_factory=...,
fixed_len=None, val_tlv_type=None, ignore_critical=False,
field_type=None, **kwargs):
metadata = {'tlv_type': tlv_type}
if fixed_len is not None: metadata['fixed_len'] = fixed_len
...
return dataclasses.field(default=default, ..., metadata=metadata, **kwargs)It still delegates to dataclasses.field, so everything about stdlib dataclasses keeps working. What you gain: IDE autocomplete on the option names, typos become immediate TypeError: unexpected keyword argument at class definition instead of silently-dropped wire fields, and the signature documents the options in one place instead of a docs paragraph that drifts from the code (as it already has). The metadata dict isn't "stdlib purity" — it's magic strings; a typed helper removes magic rather than adding it.
| +++++++++++++++ | ||
| * Replace the descriptor-based TLV model with dataclass models and ``tlv_encode``/``tlv_parse``. | ||
| * Make the PIT-token-aware application API canonical at ``ndn.app`` and remove ``ndn.appv2``. | ||
| * Remove the legacy application API, Name Tree Schema, dispatcher, segment fetcher, and cascade validator. |
There was a problem hiding this comment.
This statement is no longer accurate.
Ref: #97 #108
New wire format is longer but with advantage:
dataclasssupport, such as=,reprandasdict. Also, models can be easily encoded into JSON and other formats for human/subagents to read.Not so good points:
bytesfield could actually holdmemoryview. Need further effort to improve.