Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove auth file in dataset copy process #1480

Merged
merged 19 commits into from
Nov 14, 2022

Conversation

anda-ren
Copy link
Member

@anda-ren anda-ren commented Nov 11, 2022

Description

  • remove auth file in dataset copy process
  • make storage service configurable by user in system setting
  • storage services configured by user could automatically matche with the uris in dataset index
  • change uri pattern from schema://username:password@host@bucket/path to standered URI which is schema://username:password@host/path
  • change related docs
  • remove authName in api

Verification

Storage setting

image

Evaluations

Vewers

http://uri-auto-20221110.pre.intra.starwhale.ai/projects/3/datasets/2/versions/4/files
Toggle different versions which has different types of uri inside.

Modules

  • UI
  • Controller
  • Agent
  • Client
  • Python-SDK
  • Others

Checklist

  • run code format and lint check
  • add unit test
  • add necessary doc

@anda-ren anda-ren added the feature ✨ new feature label Nov 11, 2022
@anda-ren anda-ren self-assigned this Nov 11, 2022
@codecov
Copy link

codecov bot commented Nov 11, 2022

Codecov Report

Merging #1480 (d87f2d2) into main (1c0d4b9) will decrease coverage by 0.22%.
The diff coverage is 48.62%.

@@             Coverage Diff              @@
##               main    #1480      +/-   ##
============================================
- Coverage     82.05%   81.82%   -0.23%     
- Complexity     1779     1780       +1     
============================================
  Files           320      322       +2     
  Lines         15497    15506       +9     
  Branches        831      834       +3     
============================================
- Hits          12716    12688      -28     
- Misses         2451     2483      +32     
- Partials        330      335       +5     
Flag Coverage Δ
controller 75.92% <46.85%> (-0.47%) ⬇️
standalone 87.78% <81.81%> (+0.02%) ⬆️
unittests 87.78% <81.81%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
client/starwhale/base/bundle_copy.py 94.33% <ø> (+0.55%) ⬆️
...starwhale/mlops/domain/dataset/DatasetService.java 85.60% <0.00%> (ø)
...s/domain/dataset/converter/DatasetBoConverter.java 0.00% <ø> (ø)
...n/CompatibleStorageAccessServiceBuilderAliyun.java 0.00% <0.00%> (ø)
...io/CompatibleStorageAccessServiceBuilderMinio.java 0.00% <0.00%> (ø)
...it/s3/CompatibleStorageAccessServiceBuilderS3.java 0.00% <0.00%> (ø)
.../mlops/storage/autofit/StorageConnectionToken.java 14.28% <14.28%> (ø)
...it/fs/CompatibleStorageAccessServiceBuilderFs.java 16.66% <16.66%> (ø)
...torage/autofit/CompatibleStorageAccessService.java 18.75% <18.75%> (ø)
...autofit/fs/CompatibleStorageAccessServiceFile.java 25.00% <25.00%> (ø)
... and 17 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@anda-ren anda-ren marked this pull request as ready for review November 11, 2022 06:37
@anda-ren anda-ren changed the title WIP feat: remove auth file in dataset copy process feat: remove auth file in dataset copy process Nov 11, 2022
@jialeicui
Copy link
Contributor

LGTM

@jialeicui jialeicui self-requested a review November 14, 2022 05:45
@anda-ren anda-ren merged commit e2c9a75 into star-whale:main Nov 14, 2022
@anda-ren anda-ren deleted the auto_link_auth branch November 14, 2022 05:45
@anda-ren anda-ren restored the auto_link_auth branch November 14, 2022 05:45
access, secret = _key.split(":", 1)
else:
raise FormatError(netloc)
endpoint = r.hostname or link_auth.endpoint
Copy link
Member

Choose a reason for hiding this comment

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

endpoint will miss the port.


In [12]: from urllib.parse import urlparse

In [13]: r= urlparse("s3://username:password@127.0.0.1:8000/bucket/key")

In [14]: r.hostname
Out[14]: '127.0.0.1'

In [16]: r.port
Out[16]: 8000

Copy link
Member

Choose a reason for hiding this comment

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

endpoint = r.netloc.split("@")[-1] or link_auth.endpoint

@anda-ren anda-ren linked an issue Nov 17, 2022 that may be closed by this pull request
@anda-ren anda-ren deleted the auto_link_auth branch December 2, 2022 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ✨ new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor link auth
3 participants