Delta Lake Diff: Enable feature (#5508)

* fix hover issue when hovering over a tree-entry-row * enable Delta Lake diff feature * remove experimental delta diff button * PR fixes * change docker image publish target to lakefs-plugins * fix bug hunt bugs (#5534) * remove empty line * Delta Lake: Update Dockerfile base images to Alpine (#5546) * change the Dockerfile back to Alpine and build the Delta plugin dynamically * remove alpine-sdk from dockerfile * add alpine-sdk * add comment about the 'RUSTFLAGS=-Ctarget-feature=-crt-static' flag * change back from bullseye to alpine
treeverse · Mar 22, 2023 · 245ef46 · 245ef46
1 parent c82943b
commit 245ef46
Show file tree

Hide file tree

Showing 12 changed files with 75 additions and 120 deletions.
diff --git a/.github/workflows/docker-publish-exp-image.yaml b/.github/workflows/docker-publish-exp-image.yaml
@@ -64,7 +64,7 @@ jobs:
         uses: docker/build-push-action@v3
         with:
           context: .
-          target: lakefs
+          target: lakefs-plugins
           push: true
           platforms: linux/amd64,linux/arm64,darwin/amd64,darwin/arm64
           build-args: VERSION=${{ steps.version.outputs.tag }}

diff --git a/.github/workflows/docker-publish.yaml b/.github/workflows/docker-publish.yaml
@@ -82,7 +82,7 @@ jobs:
         uses: docker/build-push-action@v3
         with:
           context: .
-          target: lakefs
+          target: lakefs-plugins
           push: true
           platforms: linux/amd64,linux/arm64,darwin/amd64,darwin/arm64
           build-args: VERSION=${{ steps.version.outputs.tag }}

diff --git a/Dockerfile b/Dockerfile
@@ -1,12 +1,11 @@
-FROM --platform=$BUILDPLATFORM golang:1.19.2-bullseye AS build
+FROM --platform=$BUILDPLATFORM golang:1.19.2-alpine3.16 AS build
 
 ARG VERSION=dev
 
 WORKDIR /build
 
 # Packages required to build
-RUN apt-get update
-RUN apt-get install -o APT::Keep-Downloaded-Packages=false -y build-essential
+RUN apk add --no-cache build-base
 
 # Copy project deps first since they don't change often
 COPY go.mod go.sum ./
@@ -27,7 +26,8 @@ RUN --mount=type=cache,target=/root/.cache/go-build \
     go build -ldflags "-X github.com/treeverse/lakefs/pkg/version.Version=${VERSION}" -o lakectl ./cmd/lakectl
 
 # Build delta diff binary
-FROM --platform=$BUILDPLATFORM rust:1.68-bullseye AS build-delta-diff-plugin
+FROM --platform=$BUILDPLATFORM rust:1.68-alpine3.16 AS build-delta-diff-plugin
+RUN apk update && apk add build-base pkgconfig openssl-dev alpine-sdk
 RUN cargo new --bin delta-diff
 WORKDIR /delta-diff
 
@@ -36,34 +36,37 @@ COPY ./pkg/plugins/diff/delta_diff_server/Cargo.lock ./Cargo.lock
 COPY ./pkg/plugins/diff/delta_diff_server/Cargo.toml ./Cargo.toml
 
 # 3. Build only the dependencies to cache them in this layer
-RUN cargo build --release
+
+# Rust default behavior is to build a static binary (default target is <arch>-unknown-linux-musl on Alpine, and musl
+# is assumed to be static). It links to openssl statically, but these are dynamic libraries. Setting RUSTFLAGS=-Ctarget-feature=-crt-static
+# forces Rust to create a dynamic binary, despite asking for musl.
+RUN RUSTFLAGS=-Ctarget-feature=-crt-static cargo build --release
 RUN rm src/*.rs
 
 # 4. Now that the dependency is built, copy your source code
 COPY ./pkg/plugins/diff/delta_diff_server/src ./src
 
 # 5. Build for release.
 RUN rm ./target/release/deps/delta_diff*
-RUN cargo build --release
+RUN RUSTFLAGS=-Ctarget-feature=-crt-static cargo build --release
 
 # lakectl image
-FROM --platform=$BUILDPLATFORM debian:11.6-slim AS lakectl
-RUN apt-get update
-RUN apt-get install -o APT::Keep-Downloaded-Packages=false -y ca-certificates
+FROM --platform=$BUILDPLATFORM alpine:3.16.0 AS lakectl
+RUN apk add -U --no-cache ca-certificates
 WORKDIR /app
 ENV PATH /app:$PATH
 COPY --from=build /build/lakectl ./
-RUN addgroup --system lakefs && adduser --system lakefs --ingroup lakefs
+RUN addgroup -S lakefs && adduser -S lakefs -G lakefs
 USER lakefs
 WORKDIR /home/lakefs
 ENTRYPOINT ["/app/lakectl"]
 
-# lakefs and lakectl image
-FROM --platform=$BUILDPLATFORM debian:11.6-slim AS lakefs
-RUN apt-get update
-RUN apt-get install -o APT::Keep-Downloaded-Packages=false -y ca-certificates
+# lakefs image
+FROM --platform=$BUILDPLATFORM alpine:3.16.0 AS lakefs-lakectl
+
+RUN apk add -U --no-cache ca-certificates
 # Be Docker compose friendly (i.e. support wait-for)
-RUN apt-get install -o APT::Keep-Downloaded-Packages=false -y netcat-openbsd
+RUN apk add netcat-openbsd
 
 WORKDIR /app
 COPY ./scripts/wait-for ./
@@ -73,19 +76,20 @@ COPY --from=build /build/lakefs /build/lakectl ./
 EXPOSE 8000/tcp
 
 # Setup user
-RUN addgroup --system lakefs && adduser --system lakefs --ingroup lakefs
+RUN addgroup -S lakefs && adduser -S lakefs -G lakefs
 USER lakefs
 WORKDIR /home/lakefs
 
 ENTRYPOINT ["/app/lakefs"]
 CMD ["run"]
 
-# lakefs, lakectl, and plugins image
-FROM --platform=$BUILDPLATFORM debian:11.6-slim AS lakefs-plugins
-RUN apt-get update
-RUN apt-get install -o APT::Keep-Downloaded-Packages=false -y ca-certificates
+# lakefs image
+FROM --platform=$BUILDPLATFORM alpine:3.16.0 AS lakefs-plugins
+
+RUN apk add -U --no-cache ca-certificates
+RUN apk add openssl-dev libc6-compat alpine-sdk
 # Be Docker compose friendly (i.e. support wait-for)
-RUN apt-get install -o APT::Keep-Downloaded-Packages=false -y netcat-openbsd
+RUN apk add netcat-openbsd
 
 WORKDIR /app
 COPY ./scripts/wait-for ./
@@ -96,11 +100,11 @@ COPY --from=build-delta-diff-plugin /delta-diff/target/release/delta_diff ./
 EXPOSE 8000/tcp
 
 # Setup user
-RUN addgroup --system lakefs && adduser --system lakefs --ingroup lakefs
+RUN addgroup -S lakefs && adduser -S lakefs -G lakefs
 USER lakefs
 WORKDIR /home/lakefs
+
 RUN mkdir -p /home/lakefs/.lakefs/plugins/diff && ln -s /app/delta_diff /home/lakefs/.lakefs/plugins/diff/delta
 
 ENTRYPOINT ["/app/lakefs"]
 CMD ["run"]
-
diff --git a/esti/ops/docker-compose-dynamodb.yaml b/esti/ops/docker-compose-dynamodb.yaml
@@ -33,7 +33,7 @@ services:
     ports:
       - "6432:8000"
   esti:
-    image: "golang:1.19.2-bullseye"
+    image: "golang:1.19.2-alpine3.16"
     links:
       - lakefs:s3.local.lakefs.io
       - lakefs:testmultipartupload.s3.local.lakefs.io

diff --git a/esti/ops/docker-compose.yaml b/esti/ops/docker-compose.yaml
@@ -37,7 +37,7 @@ services:
       POSTGRES_USER: lakefs
       POSTGRES_PASSWORD: lakefs
   esti:
-    image: "golang:1.19.2-bullseye"
+    image: "golang:1.19.2-alpine3.16"
     links:
       - lakefs:s3.local.lakefs.io
       - lakefs:testmultipartupload.s3.local.lakefs.io

diff --git a/pkg/plugins/diff/delta_diff_server/src/main.rs b/pkg/plugins/diff/delta_diff_server/src/main.rs
@@ -58,7 +58,6 @@ impl TableDiffer for DifferService {
         let diff_props: DiffProps = r.props.expect("Missing diff properties");
         let left_table_path: TablePath = diff_props.left_table_path.expect("Missing left table's path");
         let right_table_path: TablePath = diff_props.right_table_path.expect("Missing right table's path");
-
         let s3_config_map: HashMap<String, String> = utils::construct_storage_config(s3_gateway_config_req);
         let left_table_res =
             delta_ops::get_delta_table(&s3_config_map, &diff_props.repo, &left_table_path);
@@ -120,12 +119,14 @@ async fn get_diff_type(left_table_res: &Result<DeltaTable, Status>,
         (Err(status), Ok(_)) => {
             // left table wasn't found, and it doesn't exist on the base: table created
             if matches!(status.code(), Code::NotFound) {
-                if base_table_res.is_err() && matches!(base_table_res.as_ref().unwrap_err().code(), Code::NotFound) {
-                    Ok(DiffType::Created)
-                } else if base_table_res.is_ok() { // The table existed on the base branch
+                if base_table_res.is_err() {
+                    if matches!(base_table_res.as_ref().unwrap_err().code(), Code::NotFound | Code::InvalidArgument) { // didn't exist in base ref or no base ref provided
+                        Ok(DiffType::Created)
+                    } else { // There was other kind of error with the base branch
+                        Err(base_table_res.as_ref().unwrap_err().clone())
+                    }
+                } else { // The table existed on the base branch
                     Ok(DiffType::Changed)
-                } else { // There was other kind of error with the base branch
-                    Err(base_table_res.as_ref().unwrap_err().clone())
                 }
             } else {
                 Err(left_table_res.as_ref().unwrap_err().clone())
@@ -166,12 +167,13 @@ impl HistoryAndVersion {
     }
 }
 
-fn show_history(mut hist: Vec<Map<String, Value>>, table_version: DeltaDataTypeVersion) -> Result<Vec<TableOperation>, Status> {
+fn show_history(mut hist: Vec<Map<String, Value>>, mut table_version: DeltaDataTypeVersion) -> Result<Vec<TableOperation>, Status> {
     hist.reverse();
     let mut ans: Vec<TableOperation> = Vec::with_capacity(hist.len());
     for commit in hist {
         let table_op = construct_table_op(&commit, table_version)?;
         ans.push(table_op);
+        table_version -= 1;
     }
     Ok(ans)
 }

diff --git a/webui/src/lib/components/repository/changes.jsx b/webui/src/lib/components/repository/changes.jsx
@@ -1,22 +1,18 @@
 import React, {useCallback, useEffect, useState} from "react";
 
-import {
-    ArrowLeftIcon,
-    ClockIcon, DiffIcon, InfoIcon, PlusIcon, XIcon
-} from "@primer/octicons-react";
+import {ArrowLeftIcon, ClockIcon, InfoIcon, PlusIcon, XIcon} from "@primer/octicons-react";
 
 import {useAPI, useAPIWithPagination} from "../../hooks/api";
-import {Error, ExperimentalOverlayTooltip} from "../controls";
+import {Error} from "../controls";
 import {ObjectsDiff} from "./ObjectsDiff";
 import {TreeItemType} from "../../../constants";
 import * as tablesUtil from "../../../util/tablesUtil";
 import {ObjectTreeEntryRow, PrefixTreeEntryRow, TableTreeEntryRow} from "./treeRows";
 import Alert from "react-bootstrap/Alert";
-import {ComingSoonModal} from "../modals";
 import Button from "react-bootstrap/Button";
 import Card from "react-bootstrap/Card";
 import Table from "react-bootstrap/Table";
-import {refs, statistics} from "../../api";
+import {refs} from "../../api";
 import {DeltaLakeDiff} from "./TableDiff";
 import Form from "react-bootstrap/Form";
 import Row from "react-bootstrap/Row";
@@ -44,7 +40,6 @@ export const TreeItemRow = ({ entry, repo, reference, leftDiffRefID, rightDiffRe
     const [afterUpdated, setAfterUpdated] = useState(""); // state of pagination of the item's children
     const [resultsState, setResultsState] = useState({results:[], pagination:{}}); // current retrieved children of the item
     const [diffExpanded, setDiffExpanded] = useState(false); // state of a leaf item expansion
-    const enableDeltaDiff = JSON.parse(localStorage.getItem(`enable_delta_diff`));
 
     const itemType = useTreeItemType(entry, repo, leftDiffRefID, rightDiffRefID);
 
@@ -90,7 +85,7 @@ export const TreeItemRow = ({ entry, repo, reference, leftDiffRefID, rightDiffRe
             }
         </>
 
-    } else if (itemType.type === TreeItemType.Prefix || !enableDeltaDiff) {
+    } else if (itemType.type === TreeItemType.Prefix) {
         return <>
             <PrefixTreeEntryRow key={entry.path + "entry-row"} entry={entry} dirExpanded={dirExpanded} relativeTo={relativeTo} depth={depth} onClick={() => setDirExpanded(!dirExpanded)} onRevert={onRevert} onNavigate={onNavigate} getMore={getMore} repo={repo} reference={reference}/>
             {dirExpanded && results &&
@@ -163,7 +158,6 @@ function useTreeItemType(entry, repo, leftDiffRefID, rightDiffRefID) {
  * and uncommitted changes views.
  *
  * @param results to be displayed in the changes tree container
- * @param showExperimentalDeltaDiffButton whether or not to display a delta-specific experimental feature button. TODO (Tals): remove when enabling the delta diff feature.
  * @param delimiter objects delimiter ('' or '/')
  * @param uriNavigator to navigate in the page using the changes container
  * @param leftDiffRefID commitID / branch
@@ -179,11 +173,10 @@ function useTreeItemType(entry, repo, leftDiffRefID, rightDiffRefID) {
  * @param onNavigate to be called when navigating to a prefix
  * @param onRevert to be called when an object/prefix is requested to be reverted
  */
-export const ChangesTreeContainer = ({results, showExperimentalDeltaDiffButton = false, delimiter, uriNavigator,
+export const ChangesTreeContainer = ({results, delimiter, uriNavigator,
                                          leftDiffRefID, rightDiffRefID, repo, reference, internalRefresh, prefix,
                                          getMore, loading, nextPage, setAfterUpdated, onNavigate, onRevert, setIsTableMerge,
                                          changesTreeMessage= ""}) => {
-    const enableDeltaDiff = JSON.parse(localStorage.getItem(`enable_delta_diff`));
     const [tableDiffState, setTableDiffState] = useState({isShown: false, expandedTablePath: "", expandedTableName: ""});
 
     if (results.length === 0) {
@@ -192,9 +185,7 @@ export const ChangesTreeContainer = ({results, showExperimentalDeltaDiffButton =
         </div>
     } else {
         return <div className="tree-container">
-                    {!enableDeltaDiff
-                        ? <ExperimentalDeltaDiffButton showButton={showExperimentalDeltaDiffButton}/>
-                        : tableDiffState.isShown
+                    {tableDiffState.isShown
                                 ? <Button className="action-bar"
                                           variant="secondary"
                                           disabled={false}
@@ -264,38 +255,6 @@ const onTableDiffExpansion = (entry, setTableDiffState, setIsTableMerge) => () =
     }
 }
 
-const ExperimentalDeltaDiffButton = ({showButton = false}) => {
-    const [showComingSoonModal, setShowComingSoonModal] = useState(false);
-    const sendDeltaDiffStats = async () => {
-        const deltaDiffStatEvents = [
-            {
-                "class": "experimental-feature",
-                "name": "delta-diff",
-                "count": 1,
-            }
-        ];
-        await statistics.postStatsEvents(deltaDiffStatEvents);
-    }
-
-    return <>
-        <ComingSoonModal display={showComingSoonModal}
-                         onCancel={() => setShowComingSoonModal(false)}>
-            <div>lakeFS Delta Lake tables diff is under development</div>
-        </ComingSoonModal>
-        <ExperimentalOverlayTooltip>
-            <Button className="action-bar"
-                    variant="primary"
-                    hidden={!showButton}
-                    onClick={async () => {
-                        setShowComingSoonModal(true);
-                        await sendDeltaDiffStats();
-                    }}>
-                <DiffIcon/> Compare Delta Lake tables
-            </Button>
-        </ExperimentalOverlayTooltip>
-    </>
-}
-
 export const MetadataFields = ({ metadataFields, setMetadataFields}) => {
     const onChangeKey = useCallback((i) => {
         return e => {
@@ -350,4 +309,4 @@ export const MetadataFields = ({ metadataFields, setMetadataFields}) => {
             </Button>
         </div>
     )
-}
+}
diff --git a/webui/src/lib/components/repository/treeRows.jsx b/webui/src/lib/components/repository/treeRows.jsx
@@ -104,7 +104,7 @@ export const TableTreeEntryRow = ({entry, relativeTo = "", onClickExpandDiff, de
     const diffIndicator = <DiffIndicationIcon entry={entry} rowType={TreeRowType.Table}/>
 
     const rowActions = []
-    rowActions.push(new RowAction(null, null, "Show table changes", onClickExpandDiff))
+    rowActions.push(new RowAction(null, "", "Show table changes", onClickExpandDiff))
     if (onRevert) {
         rowActions.push(new RowAction(<HistoryIcon/>, "Revert changes", null, () => {
             setShowRevertConfirm(true)

diff --git a/webui/src/pages/repositories/repository/changes.jsx b/webui/src/pages/repositories/repository/changes.jsx
@@ -1,17 +1,14 @@
 import React, {useRef, useState} from "react";
 
-import {
-    GitCommitIcon,
-    HistoryIcon,
-} from "@primer/octicons-react";
+import {GitCommitIcon, HistoryIcon,} from "@primer/octicons-react";
 
 import Modal from "react-bootstrap/Modal";
 import Form from "react-bootstrap/Form";
 
 import Alert from "react-bootstrap/Alert";
 import Button from "react-bootstrap/Button";
 
-import {refs, branches, commits} from "../../../lib/api";
+import {branches, commits, refs} from "../../../lib/api";
 import {useAPIWithPagination} from "../../../lib/hooks/api";
 import {RefContextProvider, useRefs} from "../../../lib/hooks/repo";
 import {ConfirmationModal} from "../../../lib/components/modals";
@@ -269,4 +266,4 @@ const RepositoryChangesPage = () => {
     )
 }
 
-export default RepositoryChangesPage;
+export default RepositoryChangesPage;