Skip to content

builder/remotecontext: remove CachableSource, NewCachableSource#45627

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:remove_builder_streaming
May 29, 2023
Merged

builder/remotecontext: remove CachableSource, NewCachableSource#45627
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:remove_builder_streaming

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented May 25, 2023

This type (as well as TarsumBackup), was used for the experimental --stream support for the classic builder. This feature was removed in commit 6ca3ec8 (#39983), which also removed uses of the CachableSource type.

- What I did

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah thaJeztah force-pushed the remove_builder_streaming branch 2 times, most recently from a5d2006 to 3c19135 Compare May 26, 2023 13:18
@thaJeztah thaJeztah marked this pull request as ready for review May 26, 2023 13:18
@thaJeztah thaJeztah requested a review from tonistiigi as a code owner May 26, 2023 13:18
@thaJeztah
Copy link
Member Author

thaJeztah commented May 28, 2023

@AkihiroSuda @tonistiigi ptal

@thaJeztah thaJeztah mentioned this pull request May 29, 2023
Merged
@thaJeztah
builder/remotecontext: remove CachableSource, NewCachableSource
44124ab 
This type (as well as TarsumBackup), was used for the experimental --stream
support for the classic builder. This feature was removed in commit
6ca3ec8, which also removed uses of
the CachableSource type.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the remove_builder_streaming branch from 3c19135 to 44124ab Compare May 29, 2023 14:35
@thaJeztah
Copy link
Member Author

thaJeztah commented May 29, 2023

@corhere @crazy-max PTAL

@thaJeztah thaJeztah added this to the 25.0.0 milestone May 29, 2023
@thaJeztah thaJeztah merged commit 098b0fd into moby:master May 29, 2023
@thaJeztah thaJeztah deleted the remove_builder_streaming branch May 29, 2023 17:04
@crazy-max
Copy link
Member

crazy-max commented May 29, 2023 •
edited
Loading

Not related but I see when merged a job failed: https://github.com/moby/moby/actions/runs/5113934282/jobs/9193724679#step:5:55 when trying to solve BuildKit ref (returns null):

 /usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +refs/heads/null*:refs/remotes/origin/null* +refs/tags/null*:refs/tags/null*
  The process '/usr/bin/git' failed with exit code 1
  Waiting 17 seconds before trying again
  /usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +refs/heads/null*:refs/remotes/origin/null* +refs/tags/null*:refs/tags/null*
  The process '/usr/bin/git' failed with exit code 1
  Waiting 12 seconds before trying again
  /usr/bin/git -c protocol.version=2 fetch --no-tags --prune --progress --no-recurse-submodules --depth=1 origin +refs/heads/null*:refs/remotes/origin/null* +refs/tags/null*:refs/tags/null*
  Error: The process '/usr/bin/git' failed with exit code 1

Might by an issue with GitHub API to return full sha from pseudo-ver:

moby/hack/buildkit-ref

Lines 17 to 18 in 098b0fd

# use github api to return full sha to be able to use it as ref
BUILDKIT_REF=$(curl -s "https://api.github.com/repos/${BUILDKIT_REPO}/commits/${BUILDKIT_REF}" | jq -r .sha)

Will look at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@corhere corhere corhere approved these changes

@crazy-max crazy-max crazy-max approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@tonistiigi tonistiigi Awaiting requested review from tonistiigi tonistiigi is a code owner

Assignees

No one assigned

Labels

area/builder/classic-builder Build area/builder Build impact/deprecation kind/experimental status/4-merge

Projects

None yet

Milestone

25.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@thaJeztah @crazy-max @corhere @AkihiroSuda