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

[native] Register dwrf factories in TaskManagerTest #23694

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

kgpai
Copy link
Contributor

@kgpai kgpai commented Sep 20, 2024

Description

Velox is going to require that connector factories be registered in the client and not in the Velox library. This will happen once facebookincubator/velox#8871 lands. We need to ensure that TaskManagerTest is forwards compatible to enable the above PR to land.

Motivation and Context

As described in description.

Impact

None ; This is a minor change in a test case.

Test Plan

Unit test.

Release Notes

== NO RELEASE NOTE ==

@kgpai kgpai requested a review from a team as a code owner September 20, 2024 23:35
Copy link
Contributor

@aditi-pandit aditi-pandit left a comment

Choose a reason for hiding this comment

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

Thanks @kgpai

Copy link
Contributor

@aditi-pandit aditi-pandit left a comment

Choose a reason for hiding this comment

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

@kgpai : There is a format-check error . Rest of the code looks good.

Fix  : presto_cpp/main/tests/TaskManagerTest.cpp
diff --git a/presto-native-execution/presto_cpp/main/tests/TaskManagerTest.cpp b/presto-native-execution/presto_cpp/main/tests/TaskManagerTest.cpp
index 59c6d20d41..8bd485bc20 100644
--- a/presto-native-execution/presto_cpp/main/tests/TaskManagerTest.cpp
+++ b/presto-native-execution/presto_cpp/main/tests/TaskManagerTest.cpp
@@ -29,9 +29,9 @@
 #include "velox/dwio/common/FileSink.h"
 #include "velox/dwio/common/WriterFactory.h"
 #include "velox/dwio/common/tests/utils/BatchMaker.h"
-#include "velox/dwio/dwrf/writer/Writer.h"
 #include "velox/dwio/dwrf/RegisterDwrfReader.h"
 #include "velox/dwio/dwrf/RegisterDwrfWriter.h"
+#include "velox/dwio/dwrf/writer/Writer.h"
 #include "velox/exec/Exchange.h"
 #include "velox/exec/Values.h"
 #include "velox/exec/tests/utils/PlanBuilder.h"
make: **
```* [Makefile:109: format-check] Error 1

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