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

no peer config stored in temporal state #1844

Merged
merged 34 commits into from
Jun 24, 2024
Merged

no peer config stored in temporal state #1844

merged 34 commits into from
Jun 24, 2024

Conversation

serprex
Copy link
Contributor

@serprex serprex commented Jun 15, 2024

Also api uses peer names more too instead of passing configs around

This will need two version bumps to sanitize:

  • on 0.13 user pauses their mirrors. This is important, failure to do so will break mirror when upgrading
  • after upgrading to 0.14 unpause mirror. Temporal data will be sanitized at this point
  • in 0.15 sanitization logic will be removed & protobufs can remove fields. Mirror should be paused in 0.14 to prevent replay using outdated protobufs

protos/flow.proto Outdated Show resolved Hide resolved
protos/flow.proto Outdated Show resolved Hide resolved
flow/connectors/core.go Show resolved Hide resolved
@serprex serprex force-pushed the no-tempo-peer-config branch from 1b6aa51 to 8c44128 Compare June 16, 2024 18:12
@serprex serprex force-pushed the no-tempo-peer-config branch from 640396c to 01989fb Compare June 17, 2024 22:37
@serprex serprex requested a review from Amogh-Bharadwaj June 18, 2024 13:52
@serprex serprex force-pushed the no-tempo-peer-config branch 5 times, most recently from 611f940 to cd95157 Compare June 19, 2024 17:36
@serprex serprex marked this pull request as ready for review June 19, 2024 22:45
@serprex serprex force-pushed the no-tempo-peer-config branch from 735ef78 to c2e4231 Compare June 20, 2024 20:14
@serprex serprex force-pushed the no-tempo-peer-config branch from 8c0ff58 to 348758f Compare June 21, 2024 04:05
@serprex serprex requested a review from heavycrystal June 21, 2024 13:11
SELECT name, type, options
FROM peers
WHERE name = $1`, peerName)
// TODO remove in 0.15
Copy link
Contributor

Choose a reason for hiding this comment

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

don't remove, could be used for handling config changes from UI

Copy link
Contributor Author

@serprex serprex Jun 22, 2024

Choose a reason for hiding this comment

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

must not be called from temporal, UI endpoints must update directly, so should not be in activity code

@@ -33,14 +33,6 @@ type SnapshotFlowExecution struct {
tableNameSchemaMapping map[string]*protos.TableSchema
}

type cloneTablesInput struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

why removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unnecessary

Copy link
Contributor

Choose a reason for hiding this comment

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

had implemented it because getting arguments mixed around is quite possible with this call

ui/app/mirrors/create/schema.ts Show resolved Hide resolved
serprex added a commit that referenced this pull request Jun 22, 2024
needed before 0.14 so upgrade protocol can be performed for #1844
serprex and others added 3 commits June 22, 2024 20:30
Ideally we'd handle base64/hex instead of binary strings, but this does
the bad thing faster
@serprex serprex force-pushed the no-tempo-peer-config branch from e82e66e to 6b4072d Compare June 24, 2024 15:35
@serprex serprex force-pushed the no-tempo-peer-config branch from 66a5310 to d4c020b Compare June 24, 2024 15:52
@serprex serprex merged commit 3e1ddef into main Jun 24, 2024
13 checks passed
@serprex serprex deleted the no-tempo-peer-config branch June 24, 2024 16:22
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