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

truststore reload task #686

Merged
merged 22 commits into from
Sep 9, 2024
Merged

truststore reload task #686

merged 22 commits into from
Sep 9, 2024

Conversation

Miles-Garnsey
Copy link
Member

What this PR does:

Which issue(s) this PR fixes:
Fixes #651

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CHANGELOG.md updated (not required for documentation PRs)
  • CLA Signed: DataStax CLA

@Miles-Garnsey Miles-Garnsey requested a review from a team as a code owner August 21, 2024 09:36
@burmanm
Copy link
Contributor

burmanm commented Aug 21, 2024

Try to use personal fork next time so we don't have unnecessary branches here that must be removed.

burmanm
burmanm reviewed Aug 21, 2024
View reviewed changes
.gitignore Outdated Show resolved Hide resolved
@Miles-Garnsey
Copy link
Member Author

@burmanm I have this working now, and have also accounted for the fact that this is the only sync task which may fail due to being unsupported by the targeted management API instance. Can you please let me know if you're now happy with it?

burmanm
burmanm requested changes Aug 28, 2024
View reviewed changes
Copy link
Contributor

@burmanm burmanm left a comment

Choose a reason for hiding this comment

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

Requires testing and fixing the SyncFeature

@Miles-Garnsey Miles-Garnsey force-pushed the feature/tsreload-task branch 2 times, most recently from b66f59c to d47d4a0 Compare August 29, 2024 17:06
@Miles-Garnsey
Copy link
Member Author

@burmanm I think I've now addressed all of your feedback.

@Miles-Garnsey
Rename truststore reload endpoints per Micke's suggestions.
64465e0
@Miles-Garnsey
Ensure the reload task is in the FakeExecutorServerWithDetails.
0b7c1c1
@Miles-Garnsey
Add endpoint to the right place in the tests.
26f1f78
@Miles-Garnsey
Rename several variables to reflect that we might later have client t…
a10bae9 
…s reload too.
@Miles-Garnsey
Error handling for when a pod does not support the new ts reload func…
5b4c3a8 
…tionality.
@Miles-Garnsey
Implement some of Micke's feedback.
2c985c7
@Miles-Garnsey
Add more rigorous tests.
f626539
@Miles-Garnsey
Fix all tests so we test failed features endpoint for a sync feature,…
09b2e39 
… then also test that a working features endpoint gives us back the right response too.
@Miles-Garnsey
Refer to tsreload using more dsetool like language.
510e147
@Miles-Garnsey
Fix issue where returned stuff from goroutine was getting missed.
a0fb94f
@Miles-Garnsey
Micke's suggestion for additional test validation.
1472387
@Miles-Garnsey
Working to get task to fail.
75c938e
@Miles-Garnsey
Ensure that waitForTaskFailed works as expected.
e55a640
@burmanm burmanm merged commit cc63a41 into master Sep 9, 2024
41 of 42 checks passed
@sync-by-unito sync-by-unito bot mentioned this pull request Sep 17, 2024
Closed
@burmanm burmanm deleted the feature/tsreload-task branch September 23, 2024 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@burmanm burmanm burmanm 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.

Enable tsreload from cass-operator
2 participants
@Miles-Garnsey @burmanm