Fix README inaccuracies and document confirmed-but-missing API surface - #48
Open
dan-distributive wants to merge 3 commits into
Open
Fix README inaccuracies and document confirmed-but-missing API surface#48dan-distributive wants to merge 3 commits into
dan-distributive wants to merge 3 commits into
Conversation
Verified every claim against the actual dcp/api/*.py source and the
test suite:
- job.on('readystatechange', ...) never fires; Job.on() is a thin
passthrough to the real JS EventEmitter (job.py:180-196), which
requires the exact camelCase 'readyStateChange'.
- job.exec() is documented as returning None; it actually returns the
deployed job's ID (traced through _exec()/blockify).
- paymentAccountKeystore=wallet.get() was shown as a literal parameter
default; it isn't one -- wallet.get() is a runtime fallback deep in
the underlying JS method, not a signature default.
- dcp.wallet.get()'s documented return type had a typo (Keystre) and
undersold the actual default return type (BankAccountKeystore, not
the generic base Keystore).
- job.public's documented keys included "public" instead of "name",
contradicting the very next example line.
- Removed the @dcp.distribute(...) decorator claim -- zero occurrences
anywhere in source, tests, or examples.
- Fixed four job.fs.add()/chdir() examples using curly quotes, which
are invalid Python string delimiters.
- Fixed a mismatched-quote syntax error in the addSlices() example.
- Fixed dcp.JobFs() -> dcp.JobFS() (wrong case; would AttributeError).
- Fixed my_job.exec -> my_job.exec() (missing call, so the example was
a no-op).
- fetchResults()'s range_object type referenced dcp.compute.RangeObject,
which compute.js imports but never re-exports -- swapped for the
plain-dict shape the JS side actually accepts.
- Noted that dcp.compute_do (distinct from the working dcp.compute.do)
is currently broken: compute_do.py:20 references the undefined name
computedo_js instead of compute_do_js.
Also documented confirmed-real API surface that test_job.py and
test_misc_api_existence.py exercise but the README never mentioned:
job.setPaymentAccountKeystore, setSlicePaymentOffer, requires, cancel,
resume, localExec, plus the status/work/collateResults/requirements/
serializers attributes, and -- the largest gap -- the universal .aio
async namespace that class_manager.py's make_new_class() attaches to
every wrapped module and object.
setPaymentAccountKeystore is the only newly-documented method actually invoked by a test (test_setting_payment_ks); setSlicePaymentOffer, requires, cancel, resume, and localExec are only confirmed present via hasattr() checks in test_misc_api_existence.py, whose own docstring says outright it "does not test functionality." localExec in particular is suspected not to work for Pyodide work functions. Flagged all of these as unverified rather than presenting them with the same confidence as the tested method.
Traced Job's underlying EventEmitter (dcp/common/dcp-events in the dcp monorepo) and confirmed it's deliberately case-insensitive: sanitizeEventName() lowercases every event name on both .on() and .emit(), per an explicit caseSensitiveEventNames=false default (DOM Level 2 compliant). Since bifrost2's Job.on() is a thin passthrough to that same EventEmitter, 'readystatechange' and 'readyStateChange' were always equivalent -- the earlier camelCase fix wasn't wrong, just unnecessary. Reverting to the original lowercase form. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.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
1 participant
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
Verified every claim in the README against the actual
dcp/api/*.pysource and the test suite (which turned out to be the most reliable ground-truth for what's actually implemented).Fixed inaccuracies:
job.on('readystatechange', ...)never fires --Job.on()is a thin passthrough to the real JSEventEmitter(job.py:180-196), which requires the exact camelCase'readyStateChange'.job.exec()was documented as returningNone; it actually returns the deployed job's ID (traced through_exec()/blockify).paymentAccountKeystore=wallet.get()was shown as a literal parameter default -- it isn't one;wallet.get()is a runtime fallback deep in the underlying JS method, not a signature default.dcp.wallet.get()'s documented return type had a typo (Keystre) and undersold the actual default return type (BankAccountKeystore, not the generic baseKeystore).job.public's documented keys included"public"instead of"name", contradicting the very next example line.@dcp.distribute(...)decorator claim -- zero occurrences anywhere in source, tests, or examples.job.fs.add()/chdir()examples using curly quotes, which are invalid Python string delimiters.addSlices()example.dcp.JobFs()->dcp.JobFS()(wrong case; wouldAttributeError).my_job.exec->my_job.exec()(missing call, so the example was a no-op).fetchResults()'srange_objecttype referenceddcp.compute.RangeObject, whichcompute.jsimports but never re-exports -- swapped for the plain-dict shape the JS side actually accepts.dcp.compute_do(distinct from the workingdcp.compute.do) is currently broken:compute_do.py:20references the undefined namecomputedo_jsinstead ofcompute_do_js.Documented previously-missing, confirmed-real API surface (per
test_job.py/test_misc_api_existence.py):job.setPaymentAccountKeystore,setSlicePaymentOffer,requires,cancel,resume,localExec, plus thestatus/work/collateResults/requirements/serializersattributes, and -- the largest gap -- the universal.aioasync namespace thatclass_manager.py'smake_new_class()attaches to every wrapped module and object.Test plan
pytestrequires a full DCP scheduler/worker environment this session doesn't have.