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.

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

Synchronize zookeeper clusters with UI #1888

Merged
merged 17 commits into from
Aug 21, 2024
Merged

Synchronize zookeeper clusters with UI #1888

merged 17 commits into from
Aug 21, 2024

Conversation

moscicky
Copy link
Collaborator

@moscicky moscicky commented Aug 8, 2024

Allow admins to perform storage synchronization using UI. Admin selects "primary" DC for group/topic/subscription and its content is propagated to nodes in other DCs.

After synchronization only the group that was synchronized is refreshed so consistency view shows up to date information without making an expensive check for all groups
Screenshot 2024-08-21 at 11 41 43

@moscicky moscicky changed the title Zk sync UI Synchronize zookeeper clusters with UI Aug 9, 2024
@moscicky moscicky marked this pull request as ready for review August 9, 2024 10:03
await waitFor(() => {
expectNotificationDispatched(notificationStore, {
type: 'error',
title: 'notifications.sync.failure',
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: what do You think about changing its path to "notifications.consistency.sync.failure"? maybe it will be more readable

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed

import { useNotificationsStore } from '@/store/app-notifications/useAppNotifications';

export interface UseSync {
errorMessage: Ref<Error | null | undefined>;
Copy link
Contributor

Choose a reason for hiding this comment

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

why can it be undefined?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

actually the errorMessage was not consumed anywhere, removed it entirely

import static pl.allegro.tech.hermes.test.helper.builder.SubscriptionBuilder.subscriptionWithRandomName
import static pl.allegro.tech.hermes.test.helper.builder.TopicBuilder.randomTopic

class StorageSyncSpec extends MultiZookeeperIntegrationTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Well written tests 👏

szczygiel-m
szczygiel-m previously approved these changes Aug 21, 2024
@moscicky moscicky merged commit 8057c70 into master Aug 21, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants