Skip to content

Conversation

@bpblanken
Copy link
Collaborator

@bpblanken bpblanken commented Oct 29, 2025

this is unblocked now.

@bpblanken bpblanken changed the title move clickhouse load to luigi feat: move clickhouse load to luigi Oct 29, 2025
@bpblanken bpblanken marked this pull request as ready for review November 17, 2025 15:07
@bpblanken bpblanken requested a review from a team as a code owner November 17, 2025 15:07
run_id=run_id,
**lpr.model_dump(exclude='request_type'),
)
if FeatureFlag.CLICKHOUSE_LOADER_DISABLED
Copy link
Collaborator

Choose a reason for hiding this comment

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

out of scope for this change for sure, but is there a reason to continue to support this flag?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We'll need to support the loader running for open source users until we are sure everyone has migrated from hail->clickhouse.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The seqr helm charts we are releasing are totally non-functional if you have not migrated and have been for weeks. In our release notes we told people to use the 1.45.0-hail-search-final chart if they have not yet finished the migration, but if you update to the v2 charts the assumption is that you can not continue to have a working hail search running. I do not think we should continue to support this feature in the v2 charts indefinitely

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IIRC, 1.45.0-hail-search-final does not actually include the necessary steps to actually run the migration. We added the steps and documentation in the 2.0.0 release. The 2.x.x charts don't support hail-search but they do need to fully support the mechanism to export the hail tables over.

I just realized I can actually add similar code to the migration code to what I did here, which would actually allow deprecation of the loader. I'll go do that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah that makes sense

@bpblanken bpblanken merged commit 554e5fd into main Nov 17, 2025
4 checks passed
@bpblanken bpblanken deleted the benb/move_clickhouse_load_to_luigi branch November 17, 2025 15:31
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.

3 participants