Skip to content

Conversation

@hovu96
Copy link
Contributor

@hovu96 hovu96 commented Oct 23, 2025

Summary

This PR fixes two critical bugs that were preventing recipe validation from working properly:

  1. Rate limiting (429 errors) - Added automatic retry logic with exponential backoff
  2. Model validation failure - Fixed ConnectorAction schema to allow null title field

Changes Implemented

Rate Limiting Fix (DEVP-12)

  • Added automatic retry logic for 429 (Too Many Requests) errors with exponential backoff
  • Enabled retries by default (3 attempts) with rate-limit-friendly timing (1s start, 120s max, 2x backoff)
  • Fixed validator trigger/action validation (data structure mismatch)
  • Improved data pill validation for different pill types (output, refs, project_property)
  • Added comprehensive test coverage (new test_retry_429.py)

Commit: 697322c

Model Validation Fix (DEVP-270)

  • Updated OpenAPI spec to make the title field optional in the ConnectorAction schema, matching the actual API behavior
  • Regenerated OpenAPI client models
  • Added lazy-imports>=0.3.1 dependency (required by generated client)

Commit: 5cdb309

Testing

  • ✅ All unit tests passing
  • ✅ Recipe validation now works correctly
  • ✅ Type checkers and linters passing
  • ✅ Pre-commit hooks passed

🤖 Generated with Claude Code

hovu96 and others added 2 commits October 23, 2025 15:38
Add automatic retry logic for 429 (Too Many Requests) errors with
exponential backoff to handle API rate limits during recipe validation.
Enable retries by default with 3 attempts using rate-limit-friendly
timing (1s start, 120s max, 2x backoff factor).

Fix validator trigger/action validation that was broken due to data
structure mismatch. Convert connector actions and triggers from list
format to dict format for proper validation.

Improve data pill validation to handle different pill types (output,
refs, project_property) with type-specific required field validation.

Add comprehensive test coverage for retry behavior and update existing
tests to work with new retry configuration.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Updated the OpenAPI spec to make the ConnectorAction.title field
optional and nullable, reflecting the actual API behavior where
some connector actions/triggers return null for this field.

Also added lazy-imports dependency required by the auto-generated
OpenAPI client code.

Changes:
- Updated ConnectorAction schema in workato-api-spec.yaml
- Added lazy-imports>=0.3.1 to pyproject.toml dependencies
- Regenerated OpenAPI client with updated spec

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Oct 23, 2025

Coverage

Coverage Report
FileStmtsMissCoverMissing
__init__.py62296%12–13
cli
   __init__.py52394%49, 51, 60
   containers.py270100% 
cli/commands
   __init__.py00100% 
   api_clients.py286996%24, 297, 442–443, 476, 486, 576, 607, 615
   api_collections.py253398%25, 178, 340
   assets.py46295%51–52
   connections.py520599%578, 580, 586, 624, 973
   data_tables.py163596%28, 248, 262, 316–317
   guide.py166199%106
   init.py850100% 
   profiles.py1970100% 
   properties.py95198%18
   pull.py171298%190–191
   workspace.py38294%57, 67
cli/commands/connectors
   __init__.py00100% 
   command.py88297%103, 152
   connector_manager.py203498%176, 292, 300–301
cli/commands/projects
   __init__.py00100% 
   command.py2691096%353–356, 367, 433–435, 485, 489
   project_manager.py166795%48, 66, 263–264, 276, 317, 325
cli/commands/push
   __init__.py00100% 
   command.py132496%101, 104, 222, 300
cli/commands/recipes
   __init__.py00100% 
   command.py423997%113, 129–130, 267–270, 397, 703
   validator.py7062097%174, 883, 1136, 1223, 1246, 1279, 1281–1282, 1359–1361, 1457–1458, 1517–1518, 1707–1708, 1736–1738
cli/utils
   __init__.py30100% 
   exception_handler.py198696%137, 184, 211, 238, 295, 330
   gitignore.py140100% 
   ignore_patterns.py230100% 
   spinner.py430100% 
   version_checker.py135695%24, 26, 33–34, 72, 102
cli/utils/config
   __init__.py50100% 
   manager.py4541397%125, 136–138, 141, 154, 332, 456, 670, 828–829, 890, 949
   models.py330100% 
   profiles.py3091096%93, 189–190, 193, 228–230, 255–257
   workspace.py680100% 
TOTAL543312697% 

@hovu96 hovu96 requested review from j-madrone and oalami October 23, 2025 14:07
@hovu96 hovu96 self-assigned this Oct 23, 2025
@hovu96 hovu96 removed request for j-madrone and oalami October 23, 2025 14:08
- Add aiohttp-retry to pre-commit mypy additional_dependencies
- Remove unused type ignore comment for aiohttp_retry import
- Ensures both local pre-commit and GitHub Actions mypy use same config

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@hovu96 hovu96 requested a review from oalami October 23, 2025 14:17
@oalami oalami requested a review from Copilot October 23, 2025 17:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes two critical bugs preventing recipe validation from working correctly: rate limiting (429 errors) and model validation failures. The changes add automatic retry logic with exponential backoff for rate-limited requests and update the OpenAPI schema to allow null title fields in connector actions.

  • Added retry configuration with 429 status code support and exponential backoff timing
  • Fixed ConnectorAction schema to make title field optional/nullable
  • Updated validator to handle ConnectorAction objects correctly as lists rather than dicts
  • Regenerated OpenAPI client with lazy imports support

Reviewed Changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
workato-api-spec.yaml Made title field optional and nullable in ConnectorAction schema
src/workato_platform/init.py Added retry configuration function with 429 support
src/workato_platform/cli/commands/recipes/validator.py Fixed connector trigger/action handling and improved data pill validation
tests/unit/test_retry_429.py Added comprehensive tests for 429 retry configuration
tests/unit/test_workato_client.py Updated test to include retry configuration mocking
tests/unit/commands/recipes/test_validator.py Fixed test to use ConnectorAction objects instead of dicts
src/workato_platform/client/workato_api/models/connector_action.py Regenerated model with optional title field
src/workato_platform/client/workato_api/api_client.py Added UUID serialization support and regex fix
pyproject.toml Added lazy-imports dependency
.pre-commit-config.yaml Added aiohttp-retry dependency
Various generated files Regenerated OpenAPI client with new generator version

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Collaborator

@oalami oalami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • what functionality is lazy-imports providing?
  • why the use of UUID instead of keeping it as StrictStr?

I am getting this warning when running the CLI:

(workato-platform-cli) ossama@oalami-MacBook workato-platform-cli % workato
/Users/ossama/dev/workato-platform-cli/.venv/lib/python3.11/site-packages/lazy_imports/lazy_module.py:142: ShadowingWarning: ApiClient (attribute 'ApiClient' imported from module workato_platform.client.workato_api.models.api_client) shadows attribute 'ApiClient' imported from module workato_platform.client.workato_api.api_client in lazy module workato_platform.client.workato_api
  warnings.warn(
Usage: workato [OPTIONS] COMMAND [ARGS]...

  CLI tool for the Workato API

Options:
  --profile TEXT  Profile to use for authentication and region settings
  --version       Show the version and exit.
  --help          Show this message and exit.

Commands:
  api-clients      Manage API clients
  api-collections  Manage API collections (generates recipes and...
  assets           List project assets (data tables, custom connectors,...
  .....

hovu96 and others added 2 commits October 24, 2025 09:32
Changed 'customer_connectores_response' to 'custom_connectors_response'
in recipe validator to correctly reflect the variable's purpose and
improve code readability.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Upgraded openapi-generator-cli to 7.16.0 and disabled lazy imports to:
1. Eliminate the lazy-imports dependency (supply-chain security concern)
2. Resolve ShadowingWarning about ApiClient naming conflict

The lazy-imports package was causing a ShadowingWarning due to ApiClient being
imported from both workato_api.models.api_client and workato_api.api_client.
By removing lazy-imports entirely, both issues are resolved.

Changes:
- Upgraded openapi-generator-cli from 7.15.0 to 7.16.0
- Added lazyImports: false to openapi-config.yaml
- Removed lazy-imports>=0.3.1 from project dependencies
- Regenerated OpenAPI client with lazy imports disabled
- Generated code now uses direct imports instead of lazy loading

The trade-off of slightly slower import times is acceptable for our API size
in exchange for reduced dependencies and security surface area.

All tests, linting, and type checking pass.

Resolves DEVP-287: Document lazy-imports dependency purpose and usage
Resolves DEVP-289: Fix ShadowingWarning from lazy-imports when running CLI

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@hovu96
Copy link
Contributor Author

hovu96 commented Oct 24, 2025

  • what functionality is lazy-imports providing?

Lazy-imports was added by OpenAPI Generator 7.15.0 to potentially improve import performance. However, the OpenAPI community raised concerns about they added a feature flag in v7.16.0+ to make lazy imports optional. That's why I just upgraded to 7.16.0+ and disabled lazy imports by setting lazyImports: false in openapi-config.yaml.

  • why the use of UUID instead of keeping it as StrictStr?

This is auto-generated code due to upgrading to 7.15.0 for cases like this:
image

I am getting this warning when running the CLI:

(workato-platform-cli) ossama@oalami-MacBook workato-platform-cli % workato
/Users/ossama/dev/workato-platform-cli/.venv/lib/python3.11/site-packages/lazy_imports/lazy_module.py:142: ShadowingWarning: ApiClient (attribute 'ApiClient' imported from module workato_platform.client.workato_api.models.api_client) shadows attribute 'ApiClient' imported from module workato_platform.client.workato_api.api_client in lazy module workato_platform.client.workato_api
  warnings.warn(
Usage: workato [OPTIONS] COMMAND [ARGS]...

  CLI tool for the Workato API

Options:
  --profile TEXT  Profile to use for authentication and region settings
  --version       Show the version and exit.
  --help          Show this message and exit.

Commands:
  api-clients      Manage API clients
  api-collections  Manage API collections (generates recipes and...
  assets           List project assets (data tables, custom connectors,...
  .....

Fixed by upgrading to v7.16.0+ and disabling lazy imports.

@hovu96 hovu96 requested a review from oalami October 24, 2025 08:49
@oalami oalami merged commit b1816f6 into main Oct 24, 2025
5 checks passed
@oalami oalami deleted the DEVP-12-and-DEVP-270 branch October 24, 2025 17:10
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.

3 participants