Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(aiven_organization_application_user_token): optional fields #1730

Merged
merged 1 commit into from
May 24, 2024
Merged

fix(aiven_organization_application_user_token): optional fields #1730

merged 1 commit into from
May 24, 2024

Conversation

byashimov
Copy link
Contributor

@byashimov byashimov commented May 22, 2024 •
edited
Loading

Rewrites aiven_organization_application_user_token resource to SDKv2:

  • replaces Number type (up to 512 bit values support) with int type
  • this resource do not need null type support, hence the plugin framework makes it just complicated
  • there are issues with token handler in the old client, uses generated client instead

@byashimov byashimov force-pushed the byashimov-fix-app-user-tokens branch 6 times, most recently from 6c82b23 to 05e76bf Compare May 22, 2024 12:55
@byashimov
Copy link
Contributor Author

tests

@byashimov byashimov force-pushed the byashimov-fix-app-user-tokens branch 3 times, most recently from 2f93296 to 2dad9b1 Compare May 22, 2024 15:23
@byashimov
Copy link
Contributor Author

There is a flaky test with deletion which will be fixed with the client update.

@byashimov byashimov marked this pull request as ready for review May 22, 2024 15:27
@byashimov byashimov requested a review from a team as a code owner May 22, 2024 15:27
@byashimov byashimov force-pushed the byashimov-fix-app-user-tokens branch 2 times, most recently from f8f7dfc to 3afbd31 Compare May 22, 2024 16:03
rriski
rriski previously approved these changes May 23, 2024
View reviewed changes
Copy link
Contributor

@rriski rriski left a comment

Choose a reason for hiding this comment

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

LGTM, one question about the CacheGenAivenClient and the related fixme comment

internal/acctest/acctest.go Show resolved Hide resolved
internal/sdkprovider/provider/provider.go Show resolved Hide resolved
internal/schemautil/schemautil.go Show resolved Hide resolved
@byashimov byashimov force-pushed the byashimov-fix-app-user-tokens branch from 3afbd31 to 92b70b4 Compare May 24, 2024 09:27
@byashimov
Copy link
Contributor Author

@rriski rebased on main, fixed changelog conflict

@byashimov byashimov enabled auto-merge (squash) May 24, 2024 10:14
@byashimov byashimov merged commit a79e280 into main May 24, 2024
11 checks passed
@byashimov byashimov deleted the byashimov-fix-app-user-tokens branch May 24, 2024 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rriski rriski rriski approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@byashimov @rriski