Skip to content

Fix async conversion issues#234

Merged
gilesknap merged 6 commits into
more-efficient-fetchingfrom
fast-fetch-fixes
Mar 27, 2026
Merged

Fix async conversion issues#234
gilesknap merged 6 commits into
more-efficient-fetchingfrom
fast-fetch-fixes

Conversation

@gilesknap
Copy link
Copy Markdown
Member

@gilesknap gilesknap commented Mar 27, 2026

Summary

  • Fix k8s _get_service_data TaskGroup iterating over dict keys instead of items, causing all k8s tests to fail with ExceptionGroup
  • Fix webbrowser.open mocked with async function, causing test_log_history failures
  • Fix async autocomplete callbacks (avail_services, avail_versions) incompatible with typer's sync-only autocompletion
  • Fix deploy failing for services not yet in argocd (first-time deploy)
  • Fix monitor stop/start/restart doing nothing after confirmation (unawaited async commands)

Test plan

  • pytest tests/ — 34 passed, 0 failed, 0 errors
  • Manual test: ec deploy for a service not yet in argocd
  • Manual test: monitor stop/start/restart via TUI

🤖 Generated with Claude Code

gilesknap and others added 5 commits March 27, 2026 09:06
The TaskGroup iterated over dict keys of self.sts_dicts instead of
items, creating duplicate tasks that exhausted mock responses. Since
k8s fetches all data in single kubectl/helm calls (unlike argocd which
needs per-app manifest fetches), TaskGroup is unnecessary here.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
webbrowser.open is sync in production but was mocked with the async
run_interactive method. Added a sync run_interactive_sync for mocking
sync callables that don't need to be awaited.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
avail_services and avail_versions were async but typer autocompletion
callbacks must be synchronous. Made them sync and use _run_async()
internally to call the async create_version_map.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
_check_description called _check_service which raised CommandError when
the service didn't exist yet. For first-time deployments, catch the
error and proceed with no existing description.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The command functions (start, stop, restart) are now async but were
called without await in the sync do_work thread worker. Wrapped with
_run_async() to properly execute the coroutine.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gilesknap
Copy link
Copy Markdown
Member Author

@OCopping I've made these fixes and tested all the functions of ec manually against p47.

I think all is well.

@OCopping
Copy link
Copy Markdown
Contributor

There is one import linting issue though

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gilesknap gilesknap merged commit e8a14eb into more-efficient-fetching Mar 27, 2026
8 checks passed
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