Skip to content

Authenticate experiment starts and stop failed submissions - #2

Open
rhit-changs wants to merge 2 commits into
mainfrom
fix/708-authenticated-start
Open

rhit-changs wants to merge 2 commits into
mainfrom
fix/708-authenticated-start

Conversation

@rhit-changs

Copy link
Copy Markdown

Summary

The upload/submit/start flow omitted the user's token from the start request and could report success after a rejected start. Upload or submission failures could also leave the flow using an invalid response or continuing to the next stage. Send the existing authenticated token on start, validate each stage's response, and return the existing failure result as soon as a stage fails.

Refs AutomatingSciencePipeline/Monorepo#708. Companion server PR: AutomatingSciencePipeline/Monorepo#711.

Changes

  • Send {id, token} in the start request using the RequestManager's existing token.
  • Disable redirects and require HTTP 2xx for upload, submission, and start. Transport failures and HTTP rejection stop the flow instead of falling through to success.
  • For upload and submission, require a JSON object containing a non-empty string fileId or expId, and reject explicit success: false. Handle malformed or deeply nested JSON without advancing to the next request. These are string-presence checks, not client-side ObjectId format validation.
  • Open the executable and manifest with context managers. Catch local file open/read failures and close file handles on success and failure paths.
  • Preserve the existing result dictionary and upload-format error message; make the upload message optional. Accept successful 2xx responses, including an empty start-response body. If submission succeeded but start failed, return failure with the submitted experiment ID so it remains identifiable.

Validation

Local validation was recorded at 0d11241.

python -m unittest -v glados_cli_tests test_authenticated_start

  • 35 tests passed: 26 existing tests and 9 targeted tests. Parameterized failure cases are subtests within those nine tests, not additional top-level tests.
  • The targeted suite checks token propagation, empty-body start success, 201 upload/submission success, optional upload messages, redirects, HTTP errors, transport errors, invalid JSON, missing/blank/non-string response IDs, explicit failure responses, and local file failures.
  • Assertions verify that upload failure prevents submission/start, submission failure prevents start, handles are closed, and start failure preserves the submitted ID.
  • The combined run used an isolated temporary repository copy because existing tests generate ZIP fixtures. HTTP calls were mocked; no real credentials or production endpoints were used.
  • The commit-range whitespace check passed with the repository's existing CRLF convention accounted for. Independent review found additional file-I/O and nested-JSON cases, which were fixed and rechecked.

Companion server validation

The matching Monorepo revision passed 577 frontend tests, 4 backend file-identity regressions, TypeScript checks, and lint with 0 errors/12 existing warnings. Actual local browser testing with two users completed owner and shared-copy experiments with 4 successes/0 failures each, verified shared artifact downloads, and observed original-log access change 404 -> 200 -> 404 around sharing and Unfollow.

That browser run exercised Web-to-backend-to-Runner behavior. A live Python CLI upload/submit/start run against the updated server was not performed; client protocol and failure handling were verified with mocked requests, separately from the server's CLI API suite.

Compatibility and scope

  • Coordinate release with the Monorepo authorization change. The secured start route rejects legacy requests that omit authentication; this PR supplies the required existing token.
  • A successful return means the start request was accepted, not that the experiment completed. Start failure does not roll back an already submitted experiment, and no retry or rollback workflow is added.
  • Changes are limited to this upload/submit/start path. Other commands, token storage, existing endpoint defaults, and TLS behavior remain unchanged. Cancellation API/state changes remain outside this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant