Skip to content

Properly read Git metadata when running inside workspace - #1945

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

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

Conversation

@denik

@denikdenik commented Nov 29, 2024

Copy link
Copy Markdown
Contributor

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 changed the base branch from main to DECO-24125--move-git-load-to-initializeNovember 29, 2024 11:51
@denik
denik marked this pull request as ready for review November 29, 2024 11:52
Comment threadlibs/git/info.go Outdated
Comment threadlibs/git/info.go Outdated
Comment threadlibs/git/info.go Outdated
Comment threadlibs/git/info.go Outdated
Comment threadlibs/git/info.go Outdated
Comment threadlibs/git/info.go Outdated
Comment threadlibs/git/info.go
Comment threadlibs/git/info.go Outdated
Comment threadbundle/bundle.go Outdated
Comment threadlibs/git/view.go
@denik
denikforce-pushed the DECO-24125--move-git-load-to-initialize branch from 17f284e to 84d535aCompareDecember 1, 2024 20:28
@denik
denikforce-pushed the DECO-24125--fetch-git-details branch from 8729c19 to 05ff508CompareDecember 1, 2024 20:30
Comment threadbundle/bundle.go Outdated
Comment threadbundle/config/mutator/load_git_details.go
Comment threadlibs/git/fileset.go Outdated
Comment threadlibs/git/info.go
Comment threadlibs/git/info.go Outdated
Comment threadlibs/git/view.go
Base automatically changed from DECO-24125--move-git-load-to-initialize to mainDecember 2, 2024 09:54
@denik
denikforce-pushed the DECO-24125--fetch-git-details branch from 05ff508 to c3b02d9CompareDecember 2, 2024 10:29
@denik
denik requested a review from pieternDecember 3, 2024 08:53
Comment threadinternal/git_fetch_test.go
Comment threadinternal/git_fetch_test.go Outdated

@andrewnesterandrewnester left a comment

Copy link
Copy Markdown
Contributor

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.
Comment threadbundle/config/mutator/load_git_details.go Outdated
Comment threadbundle/config/mutator/load_git_details.go Outdated
Comment threadinternal/git_fetch_test.go Outdated
Comment threadinternal/git_fetch_test.go
Comment threadlibs/git/info.go Outdated
Comment threadlibs/git/info.go Outdated
Comment threadlibs/git/info.go
Comment threadlibs/git/info.go Outdated
@pietern

Copy link
Copy Markdown
Contributor

These are minor comments. Core of the PR LGTM!

Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Comment threadcmd/sync/sync.go Outdated
@github-actions

Copy link
Copy Markdown
Contributor

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 freeto 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

@denik@pietern@eng-dev-ecosystem-bot@andrewnester@shreyas-goenka