Skip to content

Springadds addressed a few minor issues in project files and added shared access signature test#10

Closed
dfugate wants to merge 6 commits into
Azure:devfrom
dfugate:springadds
Closed

Springadds addressed a few minor issues in project files and added shared access signature test#10
dfugate wants to merge 6 commits into
Azure:devfrom
dfugate:springadds

Conversation

@dfugate

@dfugate dfugate commented Jun 7, 2012

Copy link
Copy Markdown
Contributor

No description provided.

@tg-msft

tg-msft commented Aug 21, 2012

Copy link
Copy Markdown
Member

Changes are included in #53

@tg-msft tg-msft closed this Aug 21, 2012
lmazuel added a commit that referenced this pull request Dec 15, 2016
KieranBrantnerMagee pushed a commit to KieranBrantnerMagee/azure-sdk-for-python that referenced this pull request Mar 30, 2020
…bus/track2-tests-with-cachepreparer

track2 tests with cachepreparer, improved stress test arch
seankane-msft pushed a commit to seankane-msft/azure-sdk-for-python that referenced this pull request Jul 27, 2020
beltr0n pushed a commit to beltr0n/azure-sdk-for-python that referenced this pull request Jan 22, 2021
nagkumar91 pushed a commit to nagkumar91/azure-sdk-for-python that referenced this pull request Mar 19, 2025
sssash18 pushed a commit that referenced this pull request May 20, 2026
Build 6327280 reproduced a new failure mode on the whl + sdist CI legs
(mindependency now passes after the previous skip-marker landed):

  Playback failure -- Uri doesn't match:
    request <...?api-version=2025-08-01>
    record  <...?api-version=2025-06-01>

Root cause: my local recording environment had azure-mgmt-storage
24.0.1 (default api-version 2025-06-01); CI's whl/sdist venvs pull
the latest released azure-mgmt-storage 25.0.0 (default 2025-08-01).
URL mismatch -> cassette miss -> proxy returns 404.

Pin api_version explicitly on the cross-sub clients to the versions
captured in the recordings:
  - StorageManagementClient    -> '2025-06-01'
  - NetworkManagementClient    -> '2025-05-01'
  - AuthorizationManagementClient already pinned via the
    v2022_04_01 namespace import (no kwarg needed)

Applied to all 4 affected test methods (sync + async * #10 + #31).
The recordings themselves are unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
RaviPidaparthi pushed a commit to RaviPidaparthi/azure-sdk-for-python that referenced this pull request Jun 2, 2026
…ddressed)

Address the BLOCKING and HIGH findings from the T122 final holistic
code review.

BLOCKING fixes:

Azure#1 + Azure#2 (US7 honest accounting):
- tasks.md: T086-T093 marked [~] (deferred) to reflect that the
  per-turn / wall-clock / durable timeout semantic with persisted
  _turn_started_at is the spec target but not wired end-to-end.
- durable-task-guide.md §Timeout rewritten to honestly describe the
  current cooperative-only watchdog plus the spec target; removed
  the misleading 'remaining ≈ 2 seconds (30 - 28)' worked example
  that promised behavior the code does not provide.

Azure#3 (FR-002 Layer 3 / inline reclaim wired):
- _decorator.py::_lifecycle_start_inner: in_progress branch now
  consults _lease_is_dead (from _manager) instead of the legacy
  _in_progress_was_abandoned_legacy wall-clock heuristic. On dead
  lease: inline _reclaim_one CAS reclaim, then re-enter as
  recovered. On live-elsewhere (foreign owner): TaskConflictError
  per FR-008/Invariant 1. The legacy helper is kept as a no-op-
  returning shim only so older monkey-patches don't AttributeError.
- _manager.py::_lease_is_dead: fixed to read task_info.lease.owner
  (LeaseInfo nested object) instead of the non-existent
  task_info.lease_owner attribute. Foreign-owner records now
  correctly return False (not dead — caller observes the live-
  elsewhere conflict shape per FR-004a).

Azure#4 (SC-014 / TaskTerminated strict removal):
- _exceptions.py: TaskTerminated class fully deleted (was retained
  as transitional internal-only after Phase 9).
- __init__.py: TaskTerminated import line deleted — importing it
  from the public package now raises ImportError per SC-014.
- test_cancellation_timeout.py:
  test_task_terminated_removed_from_durable_all strengthened to
  assert ImportError + hasattr-False, not just __all__ absence.

HIGH fixes:

Azure#5 (is_superseded property removed):
- _result.py: is_superseded compatibility shim deleted entirely
  per FR-010 strict-removal contract.
- test_steering.py: 2 tests updated to assert absence of
  is_superseded via not hasattr().

Azure#7 (_steering['generation'] internal field removed):
- _decorator.py::_append_steering_input: removed
  steering['generation'] = 0 initialisation per gap-list
  §FR-021-internal.
- _manager.py::_try_drain_steering: removed
  steering['generation'] = old_generation + 1 increment. The drain
  transition IS the generation advance; no separate counter needed.
- Log line updated to remove generation references.

Test scaffolding updates:
- test_lifecycle.py::test_run_in_progress_not_stale_raises +
  test_start_follows_lifecycle_rules: seed records with foreign
  lease_owner ('other-agent|session:other-session') to exercise
  the live-elsewhere shape under the new FR-004 lease-state-based
  conflict path.

Verification:
- Full durable suite: 338 passed, 1 skipped (was 338+1 skipped;
  no regressions; deeper FR-004-based detection of dead-elsewhere
  records works against both LocalFileTaskProvider tests and the
  BindingMismatchProvider eviction tests).
- TaskTerminated importability now raises ImportError as the spec
  requires.
- is_superseded property is gone from the public class surface.

Remaining HIGH/MEDIUM items not addressed in this commit (tracked in
conformance-gap-list.md §7 for follow-up):
- HIGH Azure#6: residual terminate plumbing (slots, kwargs) in
  _ActiveTask/_run.py/_manager.py — cosmetic dead code that does not
  affect behavior.
- HIGH Azure#8: dedicated 4-cell sweep tests (SC-006/SC-008/SC-009/SC-010)
  — individual scenarios are covered by the per-area tests in place;
  the explicit parametrized sweeps are stylistic.
- HIGH Azure#9: T094(b) recovered-re-entry test + T096 queued-input
  preservation test — the behavior IS implemented (FR-027 e clause
  preserves _steering[pending_inputs] by not touching it); the
  explicit test cases are deferred.
- MEDIUM Azure#10-12: gap-list internal placeholders, lease-renewal
  eviction wiring tightening, tasks.md [~] convention rollout.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sssash18 added a commit that referenced this pull request Jun 12, 2026
* Add async and sync tests for Storage Mover management operations

- Introduced async test suite for job runs operations, verifying list and get functionalities.
- Added sync test suite for job runs operations with similar coverage.
- Created async and sync tests for project operations, including create, get, update, and delete scenarios.
- Implemented async and sync tests for storage mover operations, covering create, update, get, and delete functionalities.
- Removed outdated async and sync operation tests that were not aligned with the new structure.

* Update Storage Mover tests and configurations

- Update asset tag in assets.json for versioning.
- Enhance test sanitizers in conftest.py to avoid clobbering non-sensitive fields.
- Skip agent-related tests that require a registered VM.
- Modify job definition tests to accept scheduling variables for better flexibility.
- Adjust assertions in storage mover tests to focus on specific tags rather than total counts.

* Implement end-to-end tests for CloudToCloud job definitions with public and private sources in Storage Mover

- Added async test for starting a CloudToCloud job with a public AWS S3 bucket as the source.
- Implemented async test for starting a CloudToCloud job with a private AWS S3 bucket, including connection approval and RBAC setup.
- Enhanced existing tests to provision necessary resources and validate job execution status.
- Updated role assignment and resource cleanup logic to ensure proper teardown after tests.

* test(storagemover): add cross-sub mgmt clients to dev_requirements

Adds azure-mgmt-network, azure-mgmt-authorization, and azure-mgmt-storage
as test-only dependencies. These are required by the cross-subscription
flow exercised by matrix row #31 (test_start_c2c_job_with_private_source)
and row #10's extended public-bucket E2E (test_job_definition_job_run):
PE-connection approval on the shared PrivateLinkService, blob container
provisioning under the shared storage account, and Storage Blob Data
Contributor role assignment for the target endpoint MSI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test(storagemover): fix CI playback (sanitizer alignment + mindependency)

Two CI failures from PR #47025:

1. Playback body mismatch on the regular Build Test legs: the custom
   cross-sub sanitizer rewrote the source literal b6b34ad8-... to
   1111111-..., but the cassette already had 00000000-... (default
   AZURE_SUBSCRIPTION_ID sanitizer ran first at record time). Change
   the custom sanitizer target to 00000000-... so both sanitizers
   agree, no re-record needed. Also align role-assignment-GUID and
   principalId sanitizers to the same canonical value.

2. Collection error on mindependency/sdist legs: 'No module named
   azure.mgmt.storage.aio' — old floor-pinned azure-mgmt-storage
   lacks the .aio submodule. Guard the .aio imports in the async
   test file with pytest.importorskip so the module is skipped
   cleanly in that environment instead of failing collection.

Local validation: full suite 60 PASS / 4 SKIP / 26s in playback with
no AZURE_SUBSCRIPTION_ID env set (mimics CI).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test(storagemover): skip cross-sub sync tests on mindependency leg

Build 6327088 showed test_job_definition_job_run and
test_start_c2c_job_with_private_source still failing on the
mindependency leg with 'msrestazure.azure_exceptions.CloudError:
InvalidAuthenticationToken'. Root cause: the floor-pinned
azure-mgmt-storage (v2019_06_01) uses msrestazure transport which
bypasses azure-core's RequestsTransport, and therefore the
test-proxy, hitting live ARM instead of replaying the cassette.

Detect the modernization line via the presence of the .aio submodule
on azure-mgmt-storage/-network plus the v2022_04_01 namespace on
azure-mgmt-authorization, and gate both cross-sub sync tests behind
a pytest.mark.skipif that triggers when those signals are absent.
The matrix-#11/#12/#13 schedule tests in the same file (which do not
use cross-sub clients) are unaffected and continue to run in
mindependency.

The cross-sub imports themselves are now wrapped in try/except so the
module collects cleanly when those packages are too old to even
provide the symbols (otherwise mindependency would fail on the
from-import line before the skipif could fire).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test(storagemover): pin cross-sub mgmt client api_versions

Build 6327280 reproduced a new failure mode on the whl + sdist CI legs
(mindependency now passes after the previous skip-marker landed):

  Playback failure -- Uri doesn't match:
    request <...?api-version=2025-08-01>
    record  <...?api-version=2025-06-01>

Root cause: my local recording environment had azure-mgmt-storage
24.0.1 (default api-version 2025-06-01); CI's whl/sdist venvs pull
the latest released azure-mgmt-storage 25.0.0 (default 2025-08-01).
URL mismatch -> cassette miss -> proxy returns 404.

Pin api_version explicitly on the cross-sub clients to the versions
captured in the recordings:
  - StorageManagementClient    -> '2025-06-01'
  - NetworkManagementClient    -> '2025-05-01'
  - AuthorizationManagementClient already pinned via the
    v2022_04_01 namespace import (no kwarg needed)

Applied to all 4 affected test methods (sync + async * #10 + #31).
The recordings themselves are unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Suyash Choudhary <choudharysu@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
RaviPidaparthi added a commit that referenced this pull request Jun 18, 2026
…pec 024 Phase 5)

Lands the §A APPROVED set of public-surface simplifications in a single
coherent commit per Principle XII §4 non-duplication. Closes Phase 5
of spec 024 (responses re-design).

## Approved proposals applied
- #4 — `max_pending` option DELETED.
- #5 — `context.is_shutdown_requested` property DELETED (subsumed by
  #11's new `context.shutdown: asyncio.Event`).
- #6 + #10 — `context.durability.*` flattened onto `ResponseContext`:
  `is_recovery`, `is_steered_turn`, `pending_input_count`,
  `durable_metadata` (typed as the new public
  `DurableMetadataNamespace` Protocol).
- #8 — `store_disabled` option DELETED; composition guard
  `steerable + store_disabled` deleted with the predicate.
- #11 — cancellation surface alignment (composing causes):
  `context.cancel: asyncio.Event` + `context.shutdown: asyncio.Event`
  + `context.client_cancelled: bool`
  + `async exit_for_recovery() -> ExitForRecoverySignal`. The
  `CancellationReason` enum + `cancellation_reason` property are
  DELETED. Handler signature is hard-rejected at decoration time if
  it is not `async def` or does not take exactly 2 args.
- #12 — `replay_event_ttl_seconds` option DELETED; replaced with
  hardcoded `_REPLAY_EVENT_TTL_SECONDS = 600.0` constant in
  `hosting/_routing.py` (B35 ≥ 10 min replay verified GREEN).
- #13 — `DurabilityEntryMode` Literal alias + `entry_mode` field
  DELETED. Recovery detection is now `context.is_recovery`. The
  `_map_entry_mode` helper is replaced by `_is_recovered_entry`.

## Source changes
- `azure/ai/agentserver/responses/_options.py`: deleted parameters and
  composition guard.
- `azure/ai/agentserver/responses/_response_context.py`: flat field
  surface + composing cancellation events + `exit_for_recovery()` +
  `DurableMetadataNamespace` Protocol + `ExitForRecoverySignal` type
  alias. Class-level type annotations added so `get_type_hints()`
  and IDEs surface the precise types.
- `azure/ai/agentserver/responses/_durability_context.py`:
  `DurabilityContext` class + `DurabilityEntryMode` alias DELETED.
  `_DeveloperMetadataFacade` retained as internal impl.
- `azure/ai/agentserver/responses/__init__.py`: export
  `DurableMetadataNamespace`, `ExitForRecoverySignal`,
  `FileResponseStore`; drop `CancellationReason`.
- `azure/ai/agentserver/responses/models/runtime.py`:
  `CancellationReason` enum DELETED.
- `azure/ai/agentserver/responses/hosting/_routing.py`:
  `_validate_handler_signature()` hard-rejects sync + 3-arg handlers
  at decoration time. `_dispatch_create` invokes with 2 args.
  `_configure_streams_registry` uses the hardcoded TTL constant.
  Decorator + dispatch surface aligned with the new contract.
- `azure/ai/agentserver/responses/hosting/_endpoint_handler.py`:
  cancel-bridge sets `context.client_cancelled` / `context.shutdown`
  instead of stamping `cancellation_reason`. Disconnect monitor
  + cancel endpoint + shutdown handler all switched to the new
  composing surface. `_create_response_context` aliases
  `context.cancel` with the execution-context cancellation signal.
- `azure/ai/agentserver/responses/hosting/_durable_orchestrator.py`:
  stops constructing `DurabilityContext`; assigns flat fields
  directly on `ResponseContext`; cancel-bridge maps `ctx.shutdown` →
  `context.shutdown.set() + cancel.set()` and `ctx.cancel` (steering
  pressure) → `cancel.set()` ONLY (no cause flag).
  `_map_entry_mode` replaced by `_is_recovered_entry` boolean helper.
- `azure/ai/agentserver/responses/hosting/_orchestrator.py`: terminal
  routing reads `context.shutdown.is_set()` / `context.client_cancelled`
  instead of the deleted enum. All 3 `self._create_fn(...)` invocations
  updated to 2-arg.

## Sample updates (Principle IX)
- Samples 17, 18, 19, 20, 21, 22 (durable) all updated: 2-arg handler
  signature, `context.cancel.is_set()` cancellation observation, flat
  `context.is_recovery` / `context.durable_metadata` access, new
  shutdown-event surface via `_simulate_shutdown`.
- Samples 18 + 19 helpers (`_open_session`, `_completed_phase_index`,
  `_build_resumption_response`) take `context` instead of `durability`.
- All 17 samples import cleanly.

## Test updates
- 25-test RED suite `tests/unit/test_phase5_api_simplification.py` —
  ALL GREEN.
- Obsolete `tests/unit/test_cancellation_reason.py` +
  `tests/unit/test_durability_context.py` DELETED.
- `tests/unit/test_durable_orchestrator.py` rewritten for
  `_is_recovered_entry` + flat-context model.
- Bulk-conversion script applied across `tests/contract/`,
  `tests/integration/`, `tests/e2e/`: 3-arg → 2-arg handler signatures,
  `cancellation_signal.X` → `context.cancel.X`,
  `context.cancellation_reason == X` → cause-boolean checks,
  `context.durability.X` → flat field equivalents.
- Durability-contract harness (`tests/e2e/durability_contract/`)
  updated to drop `store_disabled` env knob and pass flat-context
  semantics through.

## Final test results
- Unit: 617/617 GREEN.
- Contract: 372/377 GREEN (5 pre-existing baseline failures:
  streaming-persistence-failure + stream-disconnect — unchanged from
  Phase 4 baseline; addressed by Phase 7 conformance gap closure).
- Integration: 39/39 GREEN.
- Interop: 62/62 GREEN.
- E2e (excluding hosted-only): 188/189 GREEN (1 skip).
- Durability-contract suite: 37/37 GREEN.
- Total: 1315/1320 GREEN (5 pre-existing baseline).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
RaviPidaparthi added a commit that referenced this pull request Jun 18, 2026
Closes all BLOCKERS and most CONCERNS / MISSING IMPLEMENTATIONS
surfaced by the final rubber-duck self-audit pass.

## BLOCKERS fixed

### Version bump 1.0.0b7 → 1.0.0b8
- ``_version.py``: VERSION constant bumped.
- ``CHANGELOG.md``: section heading bumped to 1.0.0b8 (Unreleased).
  Spec 024 mandated this bump; pre-audit it was missed.

### B17 contract conformance (non-bg disconnect persists cancelled)
- ``_orchestrator.py`` (sync background path): on non-bg + client
  disconnect, persist a ``cancelled`` snapshot via
  ``update_response`` and leave the in-memory record in cancelled
  state instead of deleting from the provider + evicting. With
  ``store=true`` GET now returns 200 + status=cancelled per
  behaviour-contract Rule B17. With ``store=false`` GET still
  returns 404.
- ``test_e6_disconnect_then_get_returns_not_found`` renamed +
  rewritten to ``test_e6_disconnect_then_get_returns_cancelled`` —
  the prior assertion encoded the pre-spec-024 (wrong) behaviour;
  the new assertion matches B17 + sibling
  ``test_e12_stream_disconnect_then_get_returns_cancelled``.

### Handler signature validation tightened
- ``_routing.py::_validate_handler_signature``: ``*args``-style
  handlers are now hard-rejected at decoration time with
  ``TypeError``. Pre-audit the validator silently accepted them
  (returning early), bypassing the "exactly two positional
  parameters" contract.

### SOT spec stale-field cleanup (final pass)
- ``docs/responses-durability-spec.md``: scrubbed remaining
  references to ``retry_attempt``, ``durability_ctx``,
  ``ctx.entry_mode``, ``ctx.retry_attempt``, and Python-implementation
  file paths in §21 change-discipline normative clauses. The doc
  is now fully language-agnostic and free of pre-spec-024 field
  names.

## CONCERNS addressed

### DurableMetadataNamespace Protocol widened to MutableMapping shape
- ``_response_context.py::DurableMetadataNamespace``: added
  ``__iter__``, ``__len__``, ``keys``, ``values``, ``items``,
  ``clear``, ``pop``, ``setdefault``, ``update``. Matches the
  underlying ``_DeveloperMetadataFacade``'s ``MutableMapping``
  surface — handler code that calls e.g.
  ``context.durable_metadata.clear()`` (sample 22) now typechecks
  cleanly against the Protocol annotation.

### Stale production-code comments scrubbed
- ``_durable_orchestrator.py``: dropped pre-spec-024 ``_shielded_runner``
  + "bookkeeping pattern / body / task" references from module
  docstring + ``_one_shot_response_task`` + ``_persist_crash_failed``
  docstrings. Deletion-acknowledgement comments retained.
- ``_orchestrator.py``: dropped vestigial ``_bookkeeping_noop_runner``
  comment block; clarified the post-spec-024 task-body completion
  flow in the inline comment near ``_persist_and_resolve_terminal``;
  updated the non-bg stream-interrupted shutdown-recovery comment
  to drop the "bookkeeping task" framing.

## MISSING IMPLEMENTATIONS — closed via 11-test audit-closure suite

New file ``tests/conformance/test_spec_024_audit_closure.py`` pins:

1. ``test_default_store_is_file_backed`` +
   ``test_default_store_uses_default_durable_root_when_env_unset`` —
   work item #1 (default response store is file-backed under
   ``${AGENTSERVER_DURABLE_ROOT:-~/.durable}/responses/``).
2. ``test_client_cancelled_observed_by_handler_after_cancel_endpoint`` —
   §10 cause matrix end-to-end via TestClient + polling pattern;
   verifies live ResponseContext exposes ``client_cancelled=True``
   AND ``cancel.is_set()`` AND ``shutdown.is_set()=False`` after
   the /cancel endpoint fires.
3. ``test_durable_metadata_protocol_includes_mutable_mapping_methods`` +
   ``test_concrete_metadata_facade_satisfies_protocol_at_runtime`` —
   pins the Protocol widening + runtime conformance.
4. ``test_handler_signature_rejects_var_positional`` +
   ``test_handler_signature_rejects_kwargs_only`` — pins the
   tightened validator.
5. ``test_exit_for_recovery_sentinel_propagates_through_dispatch`` —
   §10 ``context.exit_for_recovery()`` sentinel behaviour via the
   TestClient fallback path (raises RuntimeError outside a real
   durable context, proving the dispatch wired it through).
6. ``test_is_steered_turn_set_on_drain_reentry_via_orchestrator`` —
   spec 024 Proposal #10/#13 wire-up: durable orchestrator copies
   ``ctx.is_steered_turn`` to ``context.is_steered_turn`` on every
   entry; ``is_recovery`` stays False for "resumed" entries.
7. ``test_proposal_9_steerable_durable_off_does_not_raise`` +
   ``test_proposal_9_steerable_durable_off_host_constructs_cleanly`` —
   spec 024 Proposal #9 composition-guard relaxation pinned at both
   options-level and host-construction-level.

## Test results

- Unit: 619/619 GREEN
- Contract: 374/378 GREEN (4 pre-existing baseline failures —
  test_e12 stream-disconnect-then-get + the 3 streaming-persistence
  edge cases; documented as Hypercorn timing / runtime-state edge
  cases, NOT introduced by spec 024)
- Integration: 39/39 GREEN
- Interop: 62/62 GREEN
- E2e (excluding hosted): 188/189 GREEN (1 skip)
- Durability-contract: 37/37 GREEN
- Conformance: 21/21 GREEN (+11 audit-closure tests this commit)

Total: 1338/1343 passing (99.6%). +11 vs Phase 9 baseline. test_e6
moves from "asserting pre-spec-024 wrong behaviour" to GREEN under
the new B17-conformant code path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
RaviPidaparthi added a commit that referenced this pull request Jul 1, 2026
…10)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
RaviPidaparthi added a commit that referenced this pull request Jul 1, 2026
)

Bump _MAX_ATTACHMENT_SIZE_BYTES + MAX_ATTACHMENT_VALUE_BYTES to 10 MiB;
update SOT §23.7/§28a/§39 + tasks-guide references. Inline thresholds,
1 MB payload budget, and 20-attachment cap unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

2 participants