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

add a test that creates a zar space under brim #858

Merged
merged 5 commits into from
Jun 16, 2020
Merged

Conversation

mikesbrown
Copy link
Contributor

@mikesbrown mikesbrown commented Jun 4, 2020

Brim isn't fully ready to handle zar spaces yet, but this test
establishes a temporary workflow for getting zar data into a space and
making sure Brim starts. As Brim's functionality to read a Zar space
improves, this test can be modified. The purpose of this is to put
something in place that changes alongside the functionality as opposed
to having nothing until all the functionality is ready. As such, there's
not as much abstraction as there otherwise would be: much of this should
get thrown away once Brim supports Zar.

The test uses Brim as the driver for starting zqd so that the test don't
have to do zqd orchestration itself. It ingests sample.tsv as usual.
Then it uses zealot to create a second, empty space called sample.zar
and also get space metadata from space sample.tsv.brim. The test then
imports samle.tsv.brim's all.zng into sample.zar, with a small enough
chop size to ensure multiple files. The test creates a number of
indexes. Finally, it reloads Brim and ensures Brim sees the second space.

@mikesbrown mikesbrown requested a review from a team June 4, 2020 16:10
itest/tests/zar.test.js Outdated Show resolved Hide resolved
@mikesbrown mikesbrown marked this pull request as draft June 10, 2020 17:04
@mikesbrown mikesbrown marked this pull request as ready for review June 11, 2020 15:37
@mikesbrown mikesbrown requested a review from jameskerr June 11, 2020 15:37
@mikesbrown
Copy link
Contributor Author

This is ready again. The failure is brimdata/super#613 .

itest/tests/zar.test.js Outdated Show resolved Hide resolved
Copy link
Member

@jameskerr jameskerr left a comment

Choose a reason for hiding this comment

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

One comment regarding use of exec below, but makes sense to me otherwise.

Brim isn't fully ready to handle zar spaces yet, but this test
establishes a temporary workflow for getting zar data into a space and
making sure Brim starts. As Brim's functionality to read a Zar space
improves, this test can be modified. The purpose of this is to put
something in place that changes alongside the functionality as opposed
to having nothing until all the functionality is ready. As such, there's
not as much abstraction as there otherwise would be: much of this should
get thrown away once Brim supports Zar.

The test uses Brim as the driver for starting zqd so that the test don't
have to do zqd orchestration itself. It ingests sample.tsv as usual.
Then it uses zealot to create a second, empty space called sample.zar
and also get space metadata from space sample.tsv.brim. The test then
imports samle.tsv.brim's all.zng into sample.zar, with a small enough
chop size to ensure multiple files. The test creates a number of
indexes. Finally, it restarts Brim and ensures Brim restarts.
@mikesbrown mikesbrown merged commit 90a635f into master Jun 16, 2020
@mikesbrown mikesbrown deleted the zar-setup branch June 16, 2020 18:33
@jameskerr
Copy link
Member

👍

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