Skip to content

Fix setup command default issue#4172

Merged
MonkeyCanCode merged 2 commits intoapache:mainfrom
MonkeyCanCode:fix_setup_command_default_issue
Apr 14, 2026
Merged

Fix setup command default issue#4172
MonkeyCanCode merged 2 commits intoapache:mainfrom
MonkeyCanCode:fix_setup_command_default_issue

Conversation

@MonkeyCanCode
Copy link
Copy Markdown
Contributor

The current setup command was serializing optional fields such as roleArn (reported in #4167) as well as stsEndpoint (reported in #4076). Both of them are due to Command dataclasses don't support default value which making almost all fields mandatory in the constructor.

While I work on the setup command, my assumption was default value such as "" are safe to provide and server will handle them (miss on my end).

Now to fix this issue, this PR changes all Command dataclasses to use a flexible model by using Optional type hints with None as defaults. Then I use __post_init__ to ensure mandatory fields are getting update to proper default values if not been implicitly defined. As they are now optional, I then ensure we have proper tests in validation() to cover all required fields as well as using implicitly type casting to ensure mypy is happy with these changes.

Checklist

  • 🛡️ Don't disclose security issues! (contact security@apache.org)
  • 🔗 Clearly explained why the changes are needed, or linked related issues: Fixes #
  • 🧪 Added/updated tests with good coverage, or manually tested (and explained how)
  • 💡 Added comments for complex logic
  • 🧾 Updated CHANGELOG.md (if needed)
  • 📚 Updated documentation in site/content/in-dev/unreleased (if needed)

@MonkeyCanCode
Copy link
Copy Markdown
Contributor Author

@officialasishkumar thanks for your PR in #4167 and I believed this PR should addressed all of the issues in other classes as well as correct semantic for using Optional. Please take a look if we should proceed with this route.

@Zevrap-81, this PR should fix the problem which you are facing.

jbonofre
jbonofre previously approved these changes Apr 12, 2026
@github-project-automation github-project-automation bot moved this from PRs In Progress to Ready to merge in Basic Kanban Board Apr 12, 2026
Copy link
Copy Markdown
Contributor

@flyrain flyrain left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @MonkeyCanCode !

@MonkeyCanCode MonkeyCanCode merged commit 0b8699f into apache:main Apr 14, 2026
20 checks passed
@github-project-automation github-project-automation bot moved this from Ready to merge to Done in Basic Kanban Board Apr 14, 2026
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.

4 participants