You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/15 15:50:05 UTC

[GitHub] [arrow-datafusion] tustvold opened a new pull request, #2918: Run clippy with optional features

tustvold opened a new pull request, #2918:
URL: https://github.com/apache/arrow-datafusion/pull/2918

   Runs clippy in CI with optional features enabled, and fixes the issues this exposed


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] ursabot commented on pull request #2918: Run clippy with optional features

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #2918:
URL: https://github.com/apache/arrow-datafusion/pull/2918#issuecomment-1186181401

   Benchmark runs are scheduled for baseline = d0496b01a78d8f876898b401ea7cda4ee31bfa40 and contender = 36b82d7bc694b5880272c99a1a4bed6b7eed2714. 36b82d7bc694b5880272c99a1a4bed6b7eed2714 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/c4cccbd8b2aa44d29a02cee299415332...7aa7a35071504183a77dbe10a206a8fa/)
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/3b26cf252caa43ad88b27919a741f4d8...9cfdfa40ef554db88d8e41e63d940e29/)
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/122652d1cc3745d69db0b05559fc4d9d...8ac88dbb4f83464498b629b48a27a47c/)
   [Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/cbb55f187998419898d432352eb1f8f6...7a0a529c2a5d4ba4ba05f4bc846263a5/)
   Buildkite builds:
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] tustvold commented on a diff in pull request #2918: Run clippy with optional features

Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2918:
URL: https://github.com/apache/arrow-datafusion/pull/2918#discussion_r922298074


##########
datafusion/core/src/scheduler/task.rs:
##########
@@ -35,7 +35,7 @@ use std::sync::{Arc, Weak};
 use std::task::{Context, Poll};
 
 /// Spawns a [`PipelinePlan`] using the provided [`Spawner`]
-pub fn spawn_plan(plan: PipelinePlan, spawner: Spawner) -> ExecutionResults {
+pub(crate) fn spawn_plan(plan: PipelinePlan, spawner: Spawner) -> ExecutionResults {

Review Comment:
   This is a private module and so this change is practically unnecessary, but needed to placate clippy



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] tustvold merged pull request #2918: Run clippy with optional features

Posted by GitBox <gi...@apache.org>.
tustvold merged PR #2918:
URL: https://github.com/apache/arrow-datafusion/pull/2918


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on pull request #2918: Run clippy with optional features

Posted by GitBox <gi...@apache.org>.
alamb commented on PR #2918:
URL: https://github.com/apache/arrow-datafusion/pull/2918#issuecomment-1186155514

   Thank you @tustvold 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] tustvold commented on a diff in pull request #2918: Run clippy with optional features

Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2918:
URL: https://github.com/apache/arrow-datafusion/pull/2918#discussion_r922297082


##########
datafusion/common/src/pyarrow.rs:
##########
@@ -94,19 +94,14 @@ mod tests {
                 let python_path: Vec<&str> =
                     locals.get_item("python_path").unwrap().extract().unwrap();
 
-                Err(err).expect(
-                    format!(
-                        "pyarrow not found\nExecutable: {}\nPython path: {:?}\n\
+                panic!("pyarrow not found\nExecutable: {}\nPython path: {:?}\n\

Review Comment:
   This error appears to have just been being lost previously, which is probably not what was intended



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #2918: Run clippy with optional features

Posted by GitBox <gi...@apache.org>.
alamb commented on code in PR #2918:
URL: https://github.com/apache/arrow-datafusion/pull/2918#discussion_r922667294


##########
ci/scripts/rust_clippy.sh:
##########
@@ -18,4 +18,4 @@
 # under the License.
 
 set -ex
-cargo clippy --all-targets --workspace -- -D warnings
+cargo clippy --all-targets --workspace --features avro,jit,pyarrow,scheduler -- -D warnings

Review Comment:
   I tried running with 
   
   ```suggestion
   cargo clippy --all-targets --workspace --all-features -- -D warnings
   ```
   
   
   ```
   error[E0554]: `#![feature]` may not be used on the stable release channel
      --> /Users/alamb/.cargo/registry/src/github.com-1ecc6299db9ec823/packed_simd_2-0.3.8/src/lib.rs:214:1
       |
   214 | / #![feature(
   215 | |     adt_const_params,
   216 | |     repr_simd,
   217 | |     rustc_attrs,
   ...   |
   224 | |     custom_inner_attributes,
   225 | | )]
       | |__^
   ```



##########
ci/scripts/rust_clippy.sh:
##########
@@ -18,4 +18,4 @@
 # under the License.
 
 set -ex
-cargo clippy --all-targets --workspace -- -D warnings
+cargo clippy --all-targets --workspace --features avro,jit,pyarrow,scheduler -- -D warnings

Review Comment:
   I tried running with 
   
   ```suggestion
   cargo clippy --all-targets --workspace --all-features -- -D warnings
   ```
   
   and sadly it failed
   
   ```
   error[E0554]: `#![feature]` may not be used on the stable release channel
      --> /Users/alamb/.cargo/registry/src/github.com-1ecc6299db9ec823/packed_simd_2-0.3.8/src/lib.rs:214:1
       |
   214 | / #![feature(
   215 | |     adt_const_params,
   216 | |     repr_simd,
   217 | |     rustc_attrs,
   ...   |
   224 | |     custom_inner_attributes,
   225 | | )]
       | |__^
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] tustvold commented on a diff in pull request #2918: Run clippy with optional features

Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2918:
URL: https://github.com/apache/arrow-datafusion/pull/2918#discussion_r922678318


##########
ci/scripts/rust_clippy.sh:
##########
@@ -18,4 +18,4 @@
 # under the License.
 
 set -ex
-cargo clippy --all-targets --workspace -- -D warnings
+cargo clippy --all-targets --workspace --features avro,jit,pyarrow,scheduler -- -D warnings

Review Comment:
   Sadly there doesn't appear to be an except feature flag



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-datafusion] tustvold commented on a diff in pull request #2918: Run clippy with optional features

Posted by GitBox <gi...@apache.org>.
tustvold commented on code in PR #2918:
URL: https://github.com/apache/arrow-datafusion/pull/2918#discussion_r922297703


##########
datafusion/core/src/scheduler/mod.rs:
##########
@@ -221,12 +221,12 @@ fn spawn_local_fifo(task: Task) {
 }
 
 #[derive(Debug, Clone)]
-pub struct Spawner {
+pub(crate) struct Spawner {

Review Comment:
   This wasn't ever meant to be public, it was accidentally made public as a result of moving from a crate-private module. There was no way to actually construct it, so this is not an API change



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org