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

remove index option from scripts and reimplement it for backup #1282

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

haarg
Copy link
Member

@haarg haarg commented Oct 2, 2024

The index option is meant to control the index that has all of our ES data. But in the future, each document type will need to be in a separate index. So the idea of having a global configurable index doesn't make sense.

The backup script needs to be able to operate on all indexes, and works generically with any type. Reimplement an index option for it, allowing multiple indexes to be specified.

@haarg haarg requested a review from oalders October 2, 2024 13:17
@mickeyn
Copy link
Contributor

mickeyn commented Oct 2, 2024

is this change assuming we only work with cpan index? we do have multiple indices (all new types have their own index) and backup should be able to handle them.

@haarg
Copy link
Member Author

haarg commented Oct 3, 2024

With this PR, the backup script only requires the --index option, instead of also requiring the ES_SCRIPT_INDEX environment variable. None of the other scripts have any use for the --index option or the ES_SCRIPT_INDEX env var, because they are either using cpan_v1_01 or their own index that is defined in the code.

The index option is meant to control the index that has all of our ES
data. But in the future, each document type will need to be in a
separate index. So the idea of having a global configurable index
doesn't make sense.

The backup script needs to be able to operate on all indexes, and
works generically with any type. Reimplement an index option for it,
allowing multiple indexes to be specified.
@haarg haarg force-pushed the haarg/no-index-option branch from 91f07d6 to 5d7bf0b Compare October 3, 2024 23:17
@haarg
Copy link
Member Author

haarg commented Oct 4, 2024

I guess I didn't entirely answer your question. No, this isn't only assuming we only use the cpan index. It's just making it so the use of cpan_v1_01 is not configurable. This is so the rest of the code can be updated to refer directly to their index, rather than trying to use a dynamic value. I'm intending to set up some infrastructure to make it easy to switch the index/type of each of the current "types", so we can move to one type per index.

@haarg haarg merged commit 13f4036 into master Oct 4, 2024
1 check passed
@haarg haarg deleted the haarg/no-index-option branch October 4, 2024 12:30
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.

2 participants