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

feast(eso): rename topic #7029

Conversation

RyanHolstien
Copy link
Collaborator

Haven't tested, but pretty sure I got all the spots

@github-actions github-actions bot added devops PR or Issue related to DataHub backend & deployment product PR or Issue related to the DataHub UI/UX labels Jan 13, 2023
*/
record BuildIndicesHistoryEvent {
record DataHubUpgradeHistoryEvent {
Copy link
Collaborator

Choose a reason for hiding this comment

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

In future my guess is we'd at the upgrade id in here right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

(cuz currently we don't know which upgrade id this is for)

*/
record BuildIndicesHistoryEvent {
record DataHubUpgradeHistoryEvent {

/**
* Version of the build
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is version of the build necessary? In the rest of upgrade fw we have a concept of a "version of an upgrade". Would be useful to do something similar here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Forgot to change the doc here, yeah it can be version of upgrade instead. The version tag is used so that the distributed components can understand everything is up to date. If the upgrade ID is something local to GMS it can't be used unless all components can consistently generate it in the same way.

@@ -8,7 +8,7 @@
@RequiredArgsConstructor
public class WaitForBuildIndicesStep implements BootstrapStep {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: Make this an UpgradeStep if you want it to be written to MySQL after the first execution (it will also be skipped thereafter without needing to check kafka at all)

Copy link
Collaborator

@jjoyce0510 jjoyce0510 left a comment

Choose a reason for hiding this comment

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

LGTM

@jjoyce0510 jjoyce0510 merged commit f3a5ad2 into datahub-project:feat/elasticsearch-optimization-ext Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops PR or Issue related to DataHub backend & deployment product PR or Issue related to the DataHub UI/UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants