Skip to content

Properly read Git metadata when running inside workspace#1945

Merged
denik merged 25 commits intomainfrom
DECO-24125--fetch-git-details
Dec 5, 2024
Merged

Properly read Git metadata when running inside workspace#1945
denik merged 25 commits intomainfrom
DECO-24125--fetch-git-details

Conversation

@denik
Copy link
Copy Markdown
Contributor

@denik denik commented Nov 29, 2024

Changes

Since there is no .git directory in Workspace file system, we need to make an API call to api/2.0/workspace/get-status?return_git_info=true to fetch git the root of the repo, current branch, commit and origin.

Added new function FetchRepositoryInfo that either looks up and parses .git or calls remote API depending on env.

Refactor Repository/View/FileSet to accept repository root rather than calculate it. This helps because:

  • Repository is currently created in multiple places and finding the repository root is becoming relatively expensive (API call needed).
  • Repository/FileSet/View do not have access to current Bundle which is where WorkplaceClient is stored.

Tests

  • Tested manually by running "bundle validate --json" inside web terminal within Databricks env.
  • Added integration tests for the new API.

@denik denik temporarily deployed to test-trigger-is November 29, 2024 10:05 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is November 29, 2024 10:05 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is November 29, 2024 11:03 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is November 29, 2024 11:03 — with GitHub Actions Inactive
@denik denik changed the base branch from main to DECO-24125--move-git-load-to-initialize November 29, 2024 11:51
@denik denik marked this pull request as ready for review November 29, 2024 11:52
@denik denik temporarily deployed to test-trigger-is November 29, 2024 16:44 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is November 29, 2024 16:45 — with GitHub Actions Inactive
@denik denik force-pushed the DECO-24125--move-git-load-to-initialize branch from 17f284e to 84d535a Compare December 1, 2024 20:28
@denik denik force-pushed the DECO-24125--fetch-git-details branch from 8729c19 to 05ff508 Compare December 1, 2024 20:30
@denik denik temporarily deployed to test-trigger-is December 1, 2024 20:30 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 1, 2024 20:30 — with GitHub Actions Inactive
Base automatically changed from DECO-24125--move-git-load-to-initialize to main December 2, 2024 09:54
@denik denik force-pushed the DECO-24125--fetch-git-details branch from 05ff508 to c3b02d9 Compare December 2, 2024 10:29
@denik denik temporarily deployed to test-trigger-is December 2, 2024 10:29 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 2, 2024 10:29 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 2, 2024 10:30 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 2, 2024 10:31 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 2, 2024 10:50 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 2, 2024 10:50 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 2, 2024 14:02 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 2, 2024 14:02 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 2, 2024 14:45 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 3, 2024 08:53 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 3, 2024 08:53 — with GitHub Actions Inactive
@denik denik requested a review from pietern December 3, 2024 08:53
Copy link
Copy Markdown
Contributor

@andrewnester andrewnester left a comment

Choose a reason for hiding this comment

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

Approved but please take a look at integration tests failures on Windows before merging

It seems that because vfs.Path does filepath.Abs (OS-specific) transformation
that mangles the path even when it's already in good state, e.g. /Workspace/Something
and that causes the tests to fail on Windows.
@denik denik temporarily deployed to test-trigger-is December 3, 2024 11:48 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 3, 2024 11:48 — with GitHub Actions Inactive
@pietern
Copy link
Copy Markdown
Contributor

pietern commented Dec 4, 2024

These are minor comments. Core of the PR LGTM!

Co-authored-by: Pieter Noordhuis <[email protected]>
@denik denik temporarily deployed to test-trigger-is December 4, 2024 11:13 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 4, 2024 11:13 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 4, 2024 11:18 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 4, 2024 11:18 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 4, 2024 11:35 — with GitHub Actions Inactive
@denik denik temporarily deployed to test-trigger-is December 4, 2024 11:35 — with GitHub Actions Inactive
@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 5, 2024

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 1945
  • Commit SHA: 43da63bc50fd4f5a9b9da515db83ecd9249cdfe3

Checks will be approved automatically on success.

@eng-dev-ecosystem-bot
Copy link
Copy Markdown
Collaborator

Test Details: go/deco-tests/12176323188

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.

5 participants