image-syncer icon indicating copy to clipboard operation
image-syncer copied to clipboard

Solve the external call method of Client struct

Open weiqiang333 opened this issue 2 years ago • 4 comments

Pull Request Description

Describe what this PR does / why we need it

The external warehouse import "github.com/AliyunContainerService/image-syncer/pkg/client", When you want to do something, you can customize the NewSyncClient. For example, I want to provide a custom image-rsync-service

Does this pull request fix one issue?

For example, I want to provide a custom image-rsync-service

Describe how you did it

I will provide its services: https://github.com/weiqiang333/imagersync-service

Describe how to verify it

I performed local function verification

Special notes for reviews

weiqiang333 avatar May 23 '22 09:05 weiqiang333

https://github.com/weiqiang333/imagersync-service/tree/features/dev-internal-sync import github.com/weiqiang333/image-syncer v1.3.3

weiqiang333 avatar May 24 '22 09:05 weiqiang333

Codecov Report

Merging #98 (48a4384) into master (8a981fe) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #98   +/-   ##
=======================================
  Coverage   53.96%   53.96%           
=======================================
  Files           1        1           
  Lines          63       63           
=======================================
  Hits           34       34           
  Misses         24       24           
  Partials        5        5           
Flag Coverage Δ
unittests 53.96% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2dc8b55...48a4384. Read the comment docs.

codecov-commenter avatar May 27 '22 03:05 codecov-commenter

Maybe it's better to add some methods for the fields you need rather than export them all.

hhyasdf avatar May 27 '22 03:05 hhyasdf

Maybe it's better to add some methods for the fields you need rather than export them all.

If I want to implement a custom client like this https://github.com/weiqiang333/imagersync-service/blob/19a4d50d674cf8e4748700ae4119e6f38dd76009/internal/imagersync/imagersync.go#L18

Instead of the current specific NewSyncClient https://github.com/AliyunContainerService/image-syncer/blob/f0b1b8cbc2c080002cf8fdf138a68bbdbfc1aef3/pkg/client/client.go#L46

I don't think you want me to create more wheels like NewSyncClient in AliyunContainerService/image-syncer If you allow it to be exported, it will have more external extensibility Will also conform to the PKG dir attribute

weiqiang333 avatar May 31 '22 06:05 weiqiang333

Client is the entry that how image-syncer itself do image synchronization, and it's not propriate to just export all the feilds of "client.Client struct" for customization.

Now I minimize the "pkg/client" and make different functions apart, so you can use it freely.

hhyasdf avatar Jul 24 '23 02:07 hhyasdf