Repository navigation
fix: pin python deps, map encoder errors to 400 and refresh sink topics - #84
Merged
Merged
Conversation
Testing the recent dependency bumps end to end exposed problems that CI could not see: - Resolve every Python dependency from a pinned file. The NiFi and Kafka speed exporter images installed whatever was latest at build time, and CI installed unpinned packages (without confluent_kafka at all), so a green Dependabot PR did not prove the pinned version worked. Run each test suite in its own virtualenv built from the component's requirements file, and track the new files with Dependabot. - Align the NiFi scripts with asn1tools 0.169.0, the version the Event Publisher and a fresh NiFi image already use. The UPER output is byte-identical and the pyparsing deprecation warnings go away. - Return 400 instead of 500 when the request body has an unknown enumerated value or a string in an integer field: asn1tools raises KeyError/TypeError there, not EncodeError. Pin the reference DENM encoding in a test so an asn1tools bump that changes the bytes fails CI. - Refresh the MQTT sink's topic metadata every 10 s. v2x.denm.public is only created by the first POST, after the sink has subscribed to its regex, so on a fresh stack the first message waited for the default 5-minute metadata refresh before reaching MQTT. Co-Authored-By: Claude Code - Claude Opus 5.5 (medium) <noreply@anthropic.com> Signed-off-by: Luis Alfredo Perez Medina <perezmlal@outlook.com>
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.
Description
Follow-up to the dependency bumps merged today (#75, #76, #79, #81, #82). Testing them end to end, against the full Docker stack as well as in CI, turned up four problems that CI could not see. None of them was introduced by those PRs.
1. Event Publisher returns
500for some malformed bodies.asn1toolsreports most schema mismatches asEncodeError, but it raisesKeyErrorfor an unknown enumerated value (e.g."altitudeConfidence": "nope") andTypeErrorfor a string in an integer field. OnlyEncodeErrorwas handled, so these two cases returned500. Both are now caught and return400, with the exception type added to the warning log.2. On a fresh stack, the first message takes ~5 minutes to reach MQTT. The MQTT Sink connector subscribes to
topics.regexabout 100 s after start.v2x.denm.publiconly exists after the first POST, and a pattern subscription discovers new topics on its metadata refresh, which defaults to 5 minutes. The connector now setsconsumer.override.metadata.max.age.ms: 10000. This is the "first end-to-end test still fails" row in the HTTP testing guide's troubleshooting table, which used to recommend restartingconnect.3. Two Docker images install unpinned Python dependencies.
Dockerfile.nifi(pip3 install xmltodict asn1tools) and the Kafka speed exporter Dockerfile (pip install kafka-python prometheus_client) installed whatever was latest at build time. In practice, a local NiFi image still ranasn1tools0.167.0 from Docker's build cache while a fresh build got 0.169.0. Both images now install pinnedrequirements.txtfiles, which Dependabot tracks. The NiFi scripts' Poetry project is also aligned toasn1tools0.169.0, which removes 362 pyparsing deprecation warnings from the test run.4. CI does not test the pinned versions. The test job ran
pip install pytest pytest-cov flask asn1tools xmltodict prometheus_client, with no versions and withoutconfluent_kafkaat all. A green Dependabot PR therefore proved nothing about the version it bumped. Each suite now runs in its own virtualenv built from its component's requirements file. A new test also checks the reference DENM's exact UPER bytes, so anasn1toolsbump that changes the encoding fails CI.Type of change
Checklist
poetry run pytestpasses locally.ruff checkandbanditreport no new issues (see the Development guide).docs/,CHANGELOG.md).How this was verified
unknown-enum,string-for-integer)testjob replayed locally (samerun:blocks), Python 3.9 and 3.12poetry run pytestwith the new lock, Python 3.9 and 3.12ruff(locked 0.16.10 and latest),bandit,poetry check --lock,actionlintdocker compose up --buildfrom this branch, fresh volumesupasn1tools0.169.0,xmltodict1.0.4; exporter:kafka-python3.0.11,prometheus_client0.26.0metadata.max.age.ms = 10000(the override was accepted)mainearlier today)400(was500)encoder_decoder.pyinside the container (DenmEtsimulti-part)Notes for reviewers
docker-compose down && docker-compose up -d). This is noted inCHANGELOG.mdand the troubleshooting table.asn1tools0.169.0 instead of whatever the build cache holds. The output was verified byte-identical.deployment/nifi/requirements.txt(what the image installs, and what CI tests) andnifi-scripts/pyproject.toml(the Poetry dev project) list the same libraries. Dependabot updates each directory separately. This is documented indocs/Development.mdand in theAGENTS.mdwatchlist, the same way the duplicated ASN.1 schemas are.-5for an unsigned station ID, or10**40) still encode without error, becauseasn1toolsdoes not check constraints by default. Turning that on is a behavior change for clients, so it deserves a separate PR.