Repository navigation
Write delayexecution only for a property with a value - #92
Merged
Merged
Conversation
Merged
jwrosewell
marked this pull request as ready for review
November 12, 2026 21:10
This was referenced Nov 12, 2026
Merged
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.
Problem
The JSON Builder writes
delayexecutionfor every Property whose metadata has the delay execution flag set, before it reads the Property's value (jsonbundler.pylines 178 to 182 onrcat 126d3733). So a Property assigned no value, a Property whose value is a plain null and a Property that cannot be read are all still marked, and the last two get adelayexecutionentry with no entry for the Property itself.What the specification says
json-builder.md line 71, specifications at 6570727d, in the Process section's table of meta-Properties:
.NET, Java, Go and PHP write
delayexecutiononly once they have a value.rcJsonBuilderElement.cslines 759 to 785 at de7d1922, insideif (propertyValue != null)JsonBuilderElement.javalines 307 to 338 at 0b9257f4, insideif (propertyValue != null)jsonbuilder.golines 155 to 157 at 37c4592f,if ok && !isNull && pm.DelayExecutionJsonBundlerElement.phplines 216 to 220 at 5be49565, in the branch for a value that is not nullNode.js still writes it before reading the value (
jsonbundler.jslines 168 to 170 at 871e7a37), and has the same change open as 51degrees-node#66.What changed
jsonbundler.pywritesdelayexecutionafter the value is read, and only when the value is notNone(lines 243 to 247 at the head). Each way through the loop body for one Property now ends as follows.except: ... continue(lines 226 to 229) and gets nodelayexecution.valueset toNoneand gets its null andnullreason, with nodelayexecution.delayexecution.delayexecutionwhen its metadata flag is set.Records inside a list are written by
record_json, which writes nodelayexecutionand which this draft does not change. Theevidencepropertiesentry is unchanged, and the JavaScript Properties list is unchanged.The current
rc(126d373) is merged into the branch at 605db3d with GitHub's update branch, and the merge was clean.rcchangedjsonbundler.pyin two places since this draft was opened, the exclusion list (#70) and the error text with no sequence number (#101), neither of which touches the lines this draft changes. The whole difference fromrcisjsonbundler.pyand the new test.How it was verified
New test
test_jsonbundler_delayexecution.py, with one delayed Property that has a value, one assigned no value and one plain null.rcat 126d373 with the new test and nothing else: run 43299392982 fails. The test fails on its assertion,AssertionError: 'novaluedelayexecution' unexpectedly found in {'withvaluedelayexecution': True, 'withvalue': 'script', 'novaluedelayexecution': True, 'novalue': None, 'novaluenullreason': 'No script', 'plainnulldelayexecution': True}, and the core tests give 1 failed and 479 passed.605db3db55a73d08148bb0f7d18ad91e3e87ba54: run 43299555028 succeeded ontmp/pr-matrix-92x, built fromrcat 126d373, with all ten Build and Test jobs (Ubuntu and Windows, Python 3.10 to 3.14) green. Every one of the 11Setting 'PullRequestSha'lines in its job logs reads the head, Configure printed "Left out 5 macOS entries", and the new test ran and passed in the Ubuntu Python 3.10 job.Red and green ran on throwaway branches,
tmp/fixpy-delay-red-rc(af6401e, beingrcplus the test, a temporary workflow and its list of tests) andtmp/fixpy-delay-green-rc(7247ca3, being the head plus the same workflow and list). The red and green runs of 10 November were on the older base fc68b1e and are replaced by these.macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.
What remains
Nothing in this change. #91 changes the same file at lines 126 to 130, the exclusion list, away from lines 175 to 247 that this draft changes.
The last release into
main(#39) merged on 2026-11-04.