Skip to content

Implement transforms for VAST #1517

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

Merged
merged 17 commits into from
May 9, 2021
Merged

Implement transforms for VAST #1517

merged 17 commits into from
May 9, 2021

Conversation

lava
Copy link
Member

@lava lava commented Apr 1, 2021

📔 Description

This adds an MVP implementation of "transforms" to VAST.

📝 Checklist

  • All user-facing changes have changelog entries.
  • The changes are reflected on docs.tenzir.com/vast, if necessary.
  • The PR description contains instructions for the reviewer, if necessary.

🎯 Review Instructions

Look at the commit overview first: Roughly the first half consists of related bugfixes and refactorings I encountered while writing the main feature, the transforms implementation itself is split by category.

@dominiklohmann dominiklohmann added the feature New functionality label Apr 6, 2021
@lava lava force-pushed the story/ch24236/transformer-actor branch 2 times, most recently from ad75a50 to 9cf7f9f Compare April 16, 2021 21:26
@dominiklohmann dominiklohmann mentioned this pull request Apr 21, 2021
2 tasks
@dominiklohmann dominiklohmann added the blocked Blocked by an (external) issue label Apr 23, 2021
@lava lava force-pushed the story/ch24236/transformer-actor branch from 6849e13 to e2899fe Compare April 27, 2021 18:37
@lava lava marked this pull request as ready for review April 27, 2021 18:37
@lava lava changed the title WIP: Implement transforms for VAST Implement transforms for VAST Apr 27, 2021
@lava lava force-pushed the story/ch24236/transformer-actor branch 2 times, most recently from a13b422 to 0402fbb Compare April 28, 2021 16:55
@dominiklohmann dominiklohmann removed the blocked Blocked by an (external) issue label Apr 29, 2021
@lava lava force-pushed the story/ch24236/transformer-actor branch from 0402fbb to 2c3b673 Compare April 29, 2021 11:52
@lava lava force-pushed the story/ch24236/transformer-actor branch 3 times, most recently from ccdaad3 to 2aa8fcf Compare April 29, 2021 16:15
// [...]
// };

#if VAST_ENABLE_ARROW > 0
Copy link
Member Author

Choose a reason for hiding this comment

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

This can be rewritten as #if VAST_ENABLE_ARROW

Copy link
Member

Choose a reason for hiding this comment

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

When doing this please grep for VAST_ENABLE_ARROW > 0 generally. There are a few more places.

#include "vast/system/transformer.hpp"
#include "vast/table_slice.hpp"
#include "vast/table_slice_builder_factory.hpp"
#include "vast/type_set.hpp"
Copy link
Member Author

Choose a reason for hiding this comment

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

Unnecessary includes

Copy link
Member

@tobim tobim left a comment

Choose a reason for hiding this comment

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

Fixing the Banners.

@lava lava force-pushed the story/ch24236/transformer-actor branch from 77a039e to 5c28955 Compare May 7, 2021 14:22
@lava lava force-pushed the story/ch24236/transformer-actor branch from 4e80dec to 36089ab Compare May 7, 2021 14:48
Copy link
Member

@tobim tobim left a comment

Choose a reason for hiding this comment

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

Accepting this now, but please add integration tests in a follow-up PR.

@lava lava merged commit 3beb84b into master May 9, 2021
@lava lava deleted the story/ch24236/transformer-actor branch May 9, 2021 23:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants