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.

Sign up for GitHub

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

Update cli Dockerfile to a newer ubuntu release, newer rust release #10638

Merged
merged 1 commit into from
May 24, 2024
Merged

Update cli Dockerfile to a newer ubuntu release, newer rust release #10638

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions datafusion-cli/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.

FROM rust:1.73-bullseye as builder
FROM rust:1.78-bookworm as builder
Copy link
Contributor

Choose a reason for hiding this comment

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

The workspace.package in the Cargo.toml is still 1.73. https://github.com/apache/datafusion/blob/main/Cargo.toml#L53

I am fine with updating the Debian version, but updating the Rust version might require more thoughts or having more extensive implications cc @alamb @andygrove

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Rust version update is only for the build of the CLI. Nothing more. It's not related to the MSRV of DataFusion unless the build of the CLI is the litmus test that the MSRV we have actually builds and runs.

Copy link
Contributor

Choose a reason for hiding this comment

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

The Rust version update is only for the build of the CLI. Nothing more. It's not related to the MSRV of DataFusion unless the build of the CLI is the litmus test that the MSRV we have actually builds and runs.

I think the MSRV is verified via a CI job:

datafusion/.github/workflows/rust.yml

Lines 556 to 584 in 4b7b5ab

msrv:
name: Verify MSRV (Min Supported Rust Version)
runs-on: ubuntu-latest
container:
image: amd64/rust
steps:
- uses: actions/checkout@v4
- name: Setup Rust toolchain
uses: ./.github/actions/setup-builder
- name: Install cargo-msrv
run: cargo install cargo-msrv
- name: Check datafusion
working-directory: datafusion/core
run: |
# If you encounter an error with any of the commands below
# it means some crate in your dependency tree has a higher
# MSRV (Min Supported Rust Version) than the one specified
# in the `rust-version` key of `Cargo.toml`. Check your
# dependencies or update the version in `Cargo.toml`
cargo msrv verify
- name: Check datafusion-substrait
working-directory: datafusion/substrait
run: cargo msrv verify
- name: Check datafusion-proto
working-directory: datafusion/proto
run: cargo msrv verify
- name: Check datafusion-cli
working-directory: datafusion-cli
run: cargo msrv verify


COPY . /usr/src/arrow-datafusion
COPY ./datafusion /usr/src/arrow-datafusion/datafusion
Expand All @@ -28,7 +28,7 @@ RUN rustup component add rustfmt

RUN cargo build --release

FROM debian:bullseye-slim
FROM debian:bookworm-slim

COPY --from=builder /usr/src/arrow-datafusion/datafusion-cli/target/release/datafusion-cli /usr/local/bin

Expand Down
Loading