Fix #40, #41, #71: make aether:install safe to re-run and truthful about the environment - #87
Open
corgab wants to merge 2 commits into
Open
Fix #40, #41, #71: make aether:install safe to re-run and truthful about the environment#87corgab wants to merge 2 commits into
corgab wants to merge 2 commits into
Conversation
…nment
The command no longer overwrites an existing config/aether.php: it keeps
the file, asks first when the terminal is interactive, and takes --force
to overwrite deliberately. The smoke test always runs on the local
simulator, never on the configured default driver, so an AETHER_DRIVER=aws
installation cannot submit a billable task; after the command creates a
virtual environment the smoke test runs through that interpreter instead
of the one configured at boot, so the documented happy path succeeds.
The SDK probe reads importlib.metadata.version("amazon-braket-sdk"), since
the braket namespace package has no __version__ and the old probe
reported NOT INSTALLED on every machine. The installed version is compared
with the floor pinned in bin/python/requirements.txt and an upgrade hint is
printed when it is too old. A failed venv creation now falls back to the
manual instructions instead of advertising an interpreter that does not
exist.
Closes #40
Closes #41
Closes #71
The upgrade hint and the manual instructions now name the configured
interpreter and the platform's venv layout instead of a bare pip, python3
and a Unix-only path. A missing interpreter makes the command fail instead
of reporting a complete installation, and a failed venv creation no longer
runs the dependency install against an interpreter that does not exist.
QuantumManager::localDriver() and bridge() accept an interpreter path, so
the smoke test after venv creation reuses the manager's own wiring rather
than a second copy of it. The tests assert that the smoke test resolves
driver('local') on the manager, cover a smoke test whose run throws and a
missing interpreter, and remove only the fake interpreters they created.
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
Three defects in
AetherInstallCommand, fixed together because they live in the same file and the same flow:publishConfig()ranvendor:publish --forceunconditionally, so re-runningaether:installsilently wiped a hand-editedconfig/aether.php.$manager->circuit()on the configured default driver, soAETHER_DRIVER=awsmade the installer submit a real, billable task; and aftercreateVenv()the smoke test still ran through the interpreter configured at boot, so the documented happy path ended in "The test circuit failed to run".checkDependencies()probedbraket.__version__, which does not exist (braketis a namespace package), so the SDK was reportedNOT INSTALLEDon every machine and the success path was unreachable.Plan
--forcefor the deliberate overwrite.importlib.metadata, compare with the floor pinned inbin/python/requirements.txt, and make every hint name the configured interpreter.Changes
{--force : Overwrite an existing config/aether.php}.publishConfig()publishes when the file is missing or--forceis given; otherwise it asksconfig/aether.php already exists. Overwrite it with the package default?in an interactive terminal and keeps the file in a non-interactive run, reportingKEPT (pass --force to overwrite).runTestCircuit(QuantumDevice $device)builds(new CircuitBuilder($device, 'local'))->qubits(1)->h(0)->measure()->run().verifyInstallation()picks the device:$manager->localDriver($venvPython)whencreateVenv()just produced an interpreter, otherwise$manager->driver('local').QuantumManager::localDriver(?string $pythonPath)andbridge(?string $pythonPath)are the single source of that wiring.createVenv()andhandleMissingDependencies()return the venv interpreter path on full success, null otherwise; the dependency install is skipped when the venv could not be created, and a failure falls back to the manual instructions instead of advertising an interpreter that does not exist.checkDependencies()probesimportlib.metadata.version("amazon-braket-sdk"), compares with the floor parsed byparseRequirementsFloor()fromrequirements.txt, and prints<version> (requires >= <floor>)plus<python> -m pip install --upgrade -r ...when too old.requirementsPath()replaces three copies of the relative path.showManualInstructions()uses the configured interpreter andvenvPythonPath(), so the commands are right on Windows and with a non-defaultpython3.FAILUREinstead of printing "Aether installation complete".Tests
AetherInstallCommandTest, driven by a fake#!/bin/shinterpreter answering--versionand the-cprobe (each test removes only the fakes it created):--force(file unchanged), overwritten with--force, published when absent, kept when the interactive confirmation is declined;requires >=and the floor when older with an upgrade hint naming the interpreter,NOT INSTALLEDplus interpreter-specific manual instructions when the probe fails;driver('local')on the manager and nothing else, withaether.default = aws;FAILURE; a missing interpreter returnsFAILUREwithout the completion banner.Local: pint passed, pest 804 tests passed.
Closes #40
Closes #41
Closes #71