Skip to content

Switch to use containerd api types for transfer#163

Merged
dmcgowan merged 1 commit intocontainerd:mainfrom
dmcgowan:use-containerd-transfer-types
Apr 23, 2026
Merged

Switch to use containerd api types for transfer#163
dmcgowan merged 1 commit intocontainerd:mainfrom
dmcgowan:use-containerd-transfer-types

Conversation

@dmcgowan
Copy link
Copy Markdown
Member

The transfer types are not in the main containerd repo, allowing callers to use the types without copying nerdbox's proto definition or importing nerdbox.

Any clients that use the transfer will need to be updated in tandem with this change.

Copilot AI review requested due to automatic review settings April 19, 2026 06:57
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the transfer stream protobuf types (ReadStream/WriteStream) from nerdbox-owned protos to the upstream github.com/containerd/containerd/api module, and removes nerdbox’s local datastream proto/generation artifacts accordingly.

Changes:

  • Switch ReadStream/WriteStream typeurl marshaling/unmarshaling to use github.com/containerd/containerd/api/types/transfer.
  • Remove the nerdbox datastream.proto and its generated datastream.pb.go.
  • Update api/next.txtpb to drop the removed proto descriptor.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
internal/transfer/types.go Uses containerd API transfer types for stream messages; keeps nerdbox type for ContainerFilesystem.
api/types/transfer/v1/datastream.pb.go Removed generated Go for stream types now sourced from containerd API.
api/proto/nerdbox/types/transfer/v1/datastream.proto Removed proto source for stream types now sourced from containerd API.
api/next.txtpb Removed the datastream proto descriptor entry.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/transfer/types.go Outdated
@dmcgowan dmcgowan marked this pull request as draft April 19, 2026 07:02
@dmcgowan dmcgowan force-pushed the use-containerd-transfer-types branch from 823a047 to d347b46 Compare April 19, 2026 07:51
The transfer types are not in the main containerd repo, allowing callers
to use the types without copying nerdbox's proto definition or importing
nerdbox.

Signed-off-by: Derek McGowan <derek@mcg.dev>
Copilot AI review requested due to automatic review settings April 21, 2026 07:31
@dmcgowan dmcgowan force-pushed the use-containerd-transfer-types branch from d347b46 to a143c93 Compare April 21, 2026 07:31
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

internal/transfer/types.go:53

  • The receiver variable name cf here is a holdover from the previous ContainerFilesystem type and is now misleading on ContainerPath. Renaming the receiver (e.g., cp) would improve readability and reduce confusion when scanning code that also deals with filesystem paths and streams.
func (cf *ContainerPath) MarshalAny(ctx context.Context, sm streaming.StreamCreator) (typeurl.Any, error) {
	return typeurl.MarshalAny(&transferpb.ContainerPath{
		ContainerID:       cf.ContainerID,
		Path:              cf.Path,
		NoWalk:            cf.NoWalk,
		PreserveOwnership: cf.PreserveOwnership,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dmcgowan dmcgowan marked this pull request as ready for review April 21, 2026 07:34
@dmcgowan dmcgowan merged commit aa1f22b into containerd:main Apr 23, 2026
16 checks passed
@dmcgowan dmcgowan deleted the use-containerd-transfer-types branch April 23, 2026 05:31
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.

3 participants