Skip to content

Conversation

@J-jxr
Copy link
Contributor

@J-jxr J-jxr commented Oct 9, 2025

Overview

This pull request refactors the Nydus-to-OCI reverse conversion workflow (originally implemented in PR #1741) by introducing explicit usage of acceleration-service and nydus-snapshotter APIs.

Related Issues

Related #1741, #1682

Change Details

  • Integrated acceleration-service APIs: Replaced custom conversion logic with calls to acceleration-service interfaces .
  • Adopted nydus-snapshotter APIs: Utilized snapshotter-specific methods for image layer validation and format conversion.
  • Removed obsolete custom logic: Eliminated handcrafted layer parsing and format conversion code that was no longer needed after adopting the third-party APIs.

Test Results

  • Validated end-to-end Nydus-to-OCI conversion functionality with the refactored logic, confirming output consistency with the original implementation.

Change Type

Code Refactoring

Self-Checklist

  • I have run a code style check and addressed any warnings/errors
  • I have added appropriate comments to my code (if applicable).

@J-jxr J-jxr closed this Oct 9, 2025
@J-jxr J-jxr reopened this Oct 9, 2025
@codecov
Copy link

codecov bot commented Oct 9, 2025

Codecov Report

❌ Patch coverage is 22.91667% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.05%. Comparing base (340b2cb) to head (2198f0a).

Files with missing lines Patch % Lines
...ontrib/nydusify/pkg/converter/reverse_converter.go 24.44% 34 Missing ⚠️
contrib/nydusify/cmd/nydusify.go 0.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1781      +/-   ##
==========================================
+ Coverage   54.87%   55.05%   +0.18%     
==========================================
  Files         199      199              
  Lines       54111    53709     -402     
  Branches    44727    44727              
==========================================
- Hits        29693    29571     -122     
+ Misses      22930    22660     -270     
+ Partials     1488     1478      -10     
Files with missing lines Coverage Δ
contrib/nydusify/cmd/nydusify.go 16.14% <0.00%> (+0.38%) ⬆️
...ontrib/nydusify/pkg/converter/reverse_converter.go 35.63% <24.44%> (+1.30%) ⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@J-jxr J-jxr force-pushed the refactor/nydus2oci-workflow branch 2 times, most recently from 181a9a6 to 2c06c71 Compare October 10, 2025 04:18
PushRetryDelay int
WithPlainHTTP bool
}
var execCommand = exec.Command //nolint:unused
Copy link
Contributor

Choose a reason for hiding this comment

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

Just delete the unused variable.

Copy link
Contributor Author

@J-jxr J-jxr Oct 14, 2025

Choose a reason for hiding this comment

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

@BraveY, thanks for the review. I kept this execCommand variable intentionally as it's reserved for unit tests later - I'll use it to mock exec.Command in test cases. Will remove it if it turns out unnecessary eventually.

@J-jxr J-jxr force-pushed the refactor/nydus2oci-workflow branch from 2c06c71 to 5e4e77e Compare October 14, 2025 13:00
@J-jxr J-jxr force-pushed the refactor/nydus2oci-workflow branch from 5e4e77e to 2198f0a Compare October 20, 2025 12:33
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.

2 participants