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/06/08 21:29:51 UTC

[GitHub] [arrow-rs] tustvold opened a new pull request, #1822: Test more feature flag combinations in CI (#1630)

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

   # Which issue does this PR close?
   
   Closes #1630
   
   # Rationale for this change
    
   We continue to have issues with various feature flag combinations resulting in compile errors. A particularly pernicious variant of this occurs you need to enable a feature of an optional dependency for tests, for example, arrow prettyprint within parquet. To do this you add the dependency as a dev-dependency, with the feature enabled. 
   
   However, the way feature resolution traditionally works, is that even when building the library crate on its own, it would take into account features enabled by the dev-dependencies. This would mask issues with feature flags. The previous solution to this has been to have `test/dependency` crates that simply depend on the library, without the dev-dependencies.
   
   # What changes are included in this PR?
   
   https://github.com/rust-lang/cargo/issues/7916 added an option to skip using dev-dependencies when resolving features, and this was stabilised in [feature resolver v2](https://doc.rust-lang.org/nightly/cargo/reference/features.html#feature-resolver-version-2) which was released in Rust 1.51. In particular with the new feature resolver
   
   > [Dev-dependencies](https://doc.rust-lang.org/nightly/cargo/reference/specifying-dependencies.html#development-dependencies) do not activate features unless building a target that needs them (like tests or examples).
   
   This PR therefore:
   
   * Switches to using the new feature resolver
   * Uses this to test different feature flag combinations in CI
   * Removes the old `test/dependency` workaround
   * Fixes the bugs this turned up
   
   Thanks to @carols10cents for the pointer :heart: 
   
   # Are there any user-facing changes?
   
   No
   


-- 
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-rs] tustvold commented on pull request #1822: Change to use `resolver v2`, test more feature flag combinations in CI, fix errors (#1630)

Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #1822:
URL: https://github.com/apache/arrow-rs/pull/1822#issuecomment-1151418759

   MIRI appears to be unhappy... :eyes: 


-- 
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-rs] tustvold commented on a diff in pull request #1822: Test more feature flag combinations in CI (#1630)

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


##########
.github/workflows/rust.yml:
##########
@@ -108,31 +108,39 @@ jobs:
 
           # run tests on all workspace members with default feature list
           cargo test
-
-          # Switch to arrow crate
-          cd arrow
-          # re-run tests on arrow crate to ensure
-          # all arrays are created correctly
-          cargo test --features=force_validate
-          cargo test --features=prettyprint
-          # run test on arrow crate with minimal set of features
-          cargo test --no-default-features
+          
+          # re-run tests on arrow crate with all supported features
+          cargo test -p arrow --features=force_validate,prettyprint

Review Comment:
   I merged these two test runs together as I couldn't see an obvious reason to separate them, and this will make CI slightly faster



##########
.github/workflows/rust.yml:
##########
@@ -108,31 +108,39 @@ jobs:
 
           # run tests on all workspace members with default feature list
           cargo test
-
-          # Switch to arrow crate
-          cd arrow
-          # re-run tests on arrow crate to ensure
-          # all arrays are created correctly
-          cargo test --features=force_validate
-          cargo test --features=prettyprint
-          # run test on arrow crate with minimal set of features
-          cargo test --no-default-features
+          
+          # re-run tests on arrow crate with all supported features
+          cargo test -p arrow --features=force_validate,prettyprint
+          
+          # Test arrow examples
           cargo run --example builders
           cargo run --example dynamic_types
           cargo run --example read_csv
           cargo run --example read_csv_infer_schema
-          cargo check --no-default-features
+          

Review Comment:
   I moved the `cargo check` logic into here as opposed to a separate job so it can benefit from the compilation already performed above.



##########
parquet/src/errors.rs:
##########
@@ -135,6 +135,7 @@ macro_rules! eof_err {
     ($fmt:expr, $($args:expr),*) => (ParquetError::EOF(format!($fmt, $($args),*)));
 }
 
+#[cfg(any(feature = "arrow", test))]

Review Comment:
   This removes a warning



##########
Cargo.toml:
##########
@@ -24,6 +24,7 @@ members = [
         "arrow-flight",
         "integration-testing",
 ]
+resolver = "2"

Review Comment:
   Enable new resolver



##########
parquet/src/record/api.rs:
##########
@@ -27,7 +27,7 @@ use crate::data_type::{ByteArray, Decimal, Int96};
 use crate::errors::{ParquetError, Result};
 use crate::schema::types::ColumnDescPtr;
 
-#[cfg(feature = "cli")]
+#[cfg(any(feature = "cli", test))]

Review Comment:
   Drive by refactor to make consistent with rest of crate which tests everything



##########
.github/workflows/rust.yml:
##########
@@ -108,31 +108,42 @@ jobs:
 
           # run tests on all workspace members with default feature list
           cargo test
-
-          # Switch to arrow crate
-          cd arrow
-          # re-run tests on arrow crate to ensure
-          # all arrays are created correctly
-          cargo test --features=force_validate
-          cargo test --features=prettyprint
-          # run test on arrow crate with minimal set of features
-          cargo test --no-default-features
+          
+          # re-run tests on arrow crate with all supported features
+          cargo test -p arrow --features=force_validate,prettyprint
+          
+          # Test arrow examples
           cargo run --example builders
           cargo run --example dynamic_types
           cargo run --example read_csv
           cargo run --example read_csv_infer_schema
-          cargo check --no-default-features
+          
+          # Test compilation of arrow library crate with different feature combinations
+          cargo check -p arrow
+          cargo check -p arrow --no-default-features
+          
+          # Test compilation of arrow targets with different feature combinations
+          cargo check -p arrow --all-targets
+          cargo check -p arrow --no-default-features --all-targets
 
-          # Switch to parquet crate
-          cd ../parquet
-          # re-run tests on parquet crate with async feature enabled
-          cargo test --features=async
-          cargo check --no-default-features
+          # re-run tests on arrow-flight with all features
+          cargo test -p arrow-flight --all-features
 
-          # Switch to arrow-flight
-          cd ../arrow-flight
-          cargo test --features=flight-sql-experimental
-          cargo check --no-default-features
+          # re-run tests on parquet crate with all features
+          cargo test -p parquet --all-features
+          
+          # Test compilation of parquet library crate with different feature combinations
+          cargo check -p parquet
+          cargo check -p parquet --no-default-features
+          cargo check -p parquet --no-default-features --features arrow
+          
+          # Test compilation of parquet targets with different feature combinations
+          cargo check -p parquet --all-targets
+          cargo check -p parquet --no-default-features --all-targets
+          cargo check -p parquet --no-default-features --features arrow --all-targets
+          
+          # Test compilation of parquet_derive macro with different feature combinations
+          cargo check -p parquet_derive

Review Comment:
   To be completely honest I'm not sure why we were testing this, parquet_derive doesn't have any dev-dependencies or feature flags, but I guess it can't hurt to be thorough



-- 
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-rs] codecov-commenter commented on pull request #1822: Test more feature flag combinations in CI (#1630)

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1822:
URL: https://github.com/apache/arrow-rs/pull/1822#issuecomment-1150786882

   # [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1822?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#1822](https://codecov.io/gh/apache/arrow-rs/pull/1822?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6d953cc) into [master](https://codecov.io/gh/apache/arrow-rs/commit/ba38ebe52f16e3e9d2d0404f938cf3069a2ab842?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ba38ebe) will **increase** coverage by `0.07%`.
   > The diff coverage is `100.00%`.
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #1822      +/-   ##
   ==========================================
   + Coverage   83.44%   83.52%   +0.07%     
   ==========================================
     Files         199      199              
     Lines       56651    56730      +79     
   ==========================================
   + Hits        47272    47383     +111     
   + Misses       9379     9347      -32     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/arrow-rs/pull/1822?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [parquet/src/errors.rs](https://codecov.io/gh/apache/arrow-rs/pull/1822/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvZXJyb3JzLnJz) | `29.62% <ø> (ø)` | |
   | [parquet/src/record/api.rs](https://codecov.io/gh/apache/arrow-rs/pull/1822/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvcmVjb3JkL2FwaS5ycw==) | `96.82% <100.00%> (+4.79%)` | :arrow_up: |
   | [parquet\_derive/src/parquet\_field.rs](https://codecov.io/gh/apache/arrow-rs/pull/1822/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldF9kZXJpdmUvc3JjL3BhcnF1ZXRfZmllbGQucnM=) | `65.53% <0.00%> (-0.46%)` | :arrow_down: |
   | [arrow/src/datatypes/datatype.rs](https://codecov.io/gh/apache/arrow-rs/pull/1822/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YXJyb3cvc3JjL2RhdGF0eXBlcy9kYXRhdHlwZS5ycw==) | `65.42% <0.00%> (-0.38%)` | :arrow_down: |
   | [parquet/src/encodings/encoding.rs](https://codecov.io/gh/apache/arrow-rs/pull/1822/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGFycXVldC9zcmMvZW5jb2RpbmdzL2VuY29kaW5nLnJz) | `93.46% <0.00%> (-0.20%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1822?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/arrow-rs/pull/1822?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ba38ebe...6d953cc](https://codecov.io/gh/apache/arrow-rs/pull/1822?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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-rs] tustvold commented on pull request #1822: Change to use `resolver v2`, test more feature flag combinations in CI, fix errors (#1630)

Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #1822:
URL: https://github.com/apache/arrow-rs/pull/1822#issuecomment-1151222622

   Setting as draft whilst I try to get CI happy


-- 
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-rs] tustvold commented on a diff in pull request #1822: Change to use `resolver v2`, test more feature flag combinations in CI, fix errors (#1630)

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


##########
.github/workflows/rust.yml:
##########
@@ -108,31 +108,39 @@ jobs:
 
           # run tests on all workspace members with default feature list
           cargo test
-
-          # Switch to arrow crate
-          cd arrow
-          # re-run tests on arrow crate to ensure
-          # all arrays are created correctly
-          cargo test --features=force_validate
-          cargo test --features=prettyprint
-          # run test on arrow crate with minimal set of features
-          cargo test --no-default-features
+          
+          # re-run tests on arrow crate with all supported features
+          cargo test -p arrow --features=force_validate,prettyprint
+          
+          # Test arrow examples
           cargo run --example builders
           cargo run --example dynamic_types
           cargo run --example read_csv
           cargo run --example read_csv_infer_schema
-          cargo check --no-default-features
+          

Review Comment:
   Working on this, running into nonsense with environment variables and caching



-- 
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-rs] tustvold commented on a diff in pull request #1822: Change to use `resolver v2`, test more feature flag combinations in CI, fix errors (#1630)

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


##########
.github/actions/setup-builder/action.yaml:
##########
@@ -25,6 +25,26 @@ inputs:
 runs:
   using: "composite"
   steps:
+    - name: Cache Cargo
+      uses: actions/cache@v3
+      with:
+        # these represent dependencies downloaded by cargo
+        # and thus do not depend on the OS, arch nor rust version.
+        #
+        # source https://github.com/actions/cache/blob/main/examples.md#rust---cargo
+        path: |
+          /usr/local/cargo/bin/
+          /usr/local/cargo/registry/index/
+          /usr/local/cargo/registry/cache/
+          /usr/local/cargo/git/db/
+        key: cargo-cache3-
+    - name: Cache Rust dependencies
+      uses: actions/cache@v3
+      with:
+        # these represent compiled steps of both dependencies and arrow
+        # and thus are specific for a particular OS, arch and rust version.
+        path: /github/home/target
+        key: ${{ runner.os }}-${{ runner.arch }}-target-cache3-${{ inputs.rust-version }}-

Review Comment:
   This is the caching logic from before, however, it will progressively degrade over time as there doesn't seem to be anything to force "freshness" :thinking: 



-- 
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-rs] tustvold merged pull request #1822: Change to use `resolver v2`, test more feature flag combinations in CI, fix errors (#1630)

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


-- 
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-rs] tustvold commented on a diff in pull request #1822: Test more feature flag combinations in CI (#1630)

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


##########
.github/workflows/rust.yml:
##########
@@ -108,31 +108,42 @@ jobs:
 
           # run tests on all workspace members with default feature list
           cargo test
-
-          # Switch to arrow crate
-          cd arrow
-          # re-run tests on arrow crate to ensure
-          # all arrays are created correctly
-          cargo test --features=force_validate
-          cargo test --features=prettyprint
-          # run test on arrow crate with minimal set of features
-          cargo test --no-default-features
+          
+          # re-run tests on arrow crate with all supported features
+          cargo test -p arrow --features=force_validate,prettyprint
+          
+          # Test arrow examples
           cargo run --example builders
           cargo run --example dynamic_types
           cargo run --example read_csv
           cargo run --example read_csv_infer_schema
-          cargo check --no-default-features
+          
+          # Test compilation of arrow library crate with different feature combinations
+          cargo check -p arrow
+          cargo check -p arrow --no-default-features
+          
+          # Test compilation of arrow targets with different feature combinations

Review Comment:
   It would theoretically be more correct to test each target individually, so that feature resolution is not impacted by other targets, in practice this is probably good enough



-- 
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-rs] tustvold commented on pull request #1822: Change to use `resolver v2`, test more feature flag combinations in CI, fix errors (#1630)

Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #1822:
URL: https://github.com/apache/arrow-rs/pull/1822#issuecomment-1151423914

   Appears MIRI version is too old to supposed namespaced deps - https://github.com/rust-lang/cargo/issues/5565
   
   Attempting to updated it in - https://github.com/apache/arrow-rs/pull/1828


-- 
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-rs] tustvold commented on pull request #1822: Change to use `resolver v2`, test more feature flag combinations in CI, fix errors (#1630)

Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #1822:
URL: https://github.com/apache/arrow-rs/pull/1822#issuecomment-1151364872

   I had to rework the caching to allow splitting up the stages into smaller steps, without having to duplicate "export CARGO_HOME". into every step. Setting these variable at the top-level caused rustup to fail to install correctly, and so rather than using `CARGO_HOME` to move cargo's location, instead we now just cache the relevant cargo paths. I moved this logic into the "Prepare Rust Build Environment" action to reduce duplication.


-- 
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-rs] alamb commented on a diff in pull request #1822: Change to use `resolver v2`, test more feature flag combinations in CI, fix errors (#1630)

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


##########
.github/workflows/rust.yml:
##########
@@ -108,31 +108,39 @@ jobs:
 
           # run tests on all workspace members with default feature list
           cargo test
-
-          # Switch to arrow crate
-          cd arrow
-          # re-run tests on arrow crate to ensure
-          # all arrays are created correctly
-          cargo test --features=force_validate
-          cargo test --features=prettyprint
-          # run test on arrow crate with minimal set of features
-          cargo test --no-default-features
+          
+          # re-run tests on arrow crate with all supported features
+          cargo test -p arrow --features=force_validate,prettyprint

Review Comment:
   I think the original rationale was to try and test without the default features (mostly I think to try and catch build errors, which this PR does in another way)



##########
.github/workflows/rust.yml:
##########
@@ -108,31 +108,39 @@ jobs:
 
           # run tests on all workspace members with default feature list
           cargo test
-
-          # Switch to arrow crate
-          cd arrow
-          # re-run tests on arrow crate to ensure
-          # all arrays are created correctly
-          cargo test --features=force_validate
-          cargo test --features=prettyprint
-          # run test on arrow crate with minimal set of features
-          cargo test --no-default-features
+          
+          # re-run tests on arrow crate with all supported features
+          cargo test -p arrow --features=force_validate,prettyprint
+          
+          # Test arrow examples
           cargo run --example builders
           cargo run --example dynamic_types
           cargo run --example read_csv
           cargo run --example read_csv_infer_schema
-          cargo check --no-default-features
+          

Review Comment:
   Maybe it would be worth making a separate named run step (as is done https://github.com/apache/arrow-rs/pull/1822/files#diff-73e17259d77e5fbef83b2bdbbe4dc40a912f807472287f7f45b77e0cbf78792dR188)
   
   >      - name: Check compilation with simd features
   
   So when/if this check fails it will be easier to figure out what is wrong 



##########
Cargo.toml:
##########
@@ -24,6 +24,7 @@ members = [
         "arrow-flight",
         "integration-testing",
 ]
+resolver = "2"

Review Comment:
   Can you please add a comment / doc link explaining what this does?



-- 
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-rs] tustvold commented on a diff in pull request #1822: Test more feature flag combinations in CI (#1630)

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


##########
parquet/Cargo.toml:
##########
@@ -39,38 +39,44 @@ brotli = { version = "3.3", optional = true }
 flate2 = { version = "1.0", optional = true }
 lz4 = { version = "1.23", optional = true }
 zstd = { version = "0.11.1", optional = true, default-features = false }
-chrono = { version = "0.4", default-features = false }
+chrono = { version = "0.4", default-features = false, features = ["alloc"] }

Review Comment:
   This is the actual fix for #1630
   
   Unfortunately this is required by the record API which is tightly coupled with the file APIs, and so there isn't an easy way to hide this behind a feature flag. Perhaps something for another day, I'm a bit feature flagged out :laughing: 



-- 
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-rs] tustvold commented on pull request #1822: Change to use `resolver v2`, test more feature flag combinations in CI, fix errors (#1630)

Posted by GitBox <gi...@apache.org>.
tustvold commented on PR #1822:
URL: https://github.com/apache/arrow-rs/pull/1822#issuecomment-1151392514

   [Here](https://github.com/apache/arrow-rs/runs/6817433083?check_suite_focus=true) is an example of it restoring from a cache key (without lockfile suffix), and then publishing the lockfile specific cache key :tada:
   
   This then gets picked up by https://github.com/apache/arrow-rs/runs/6817593817?check_suite_focus=true


-- 
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-rs] alamb commented on pull request #1822: Change to use `resolver v2`, test more feature flag combinations in CI, fix errors (#1630)

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

   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