-
Notifications
You must be signed in to change notification settings - Fork 1.3k
remote: add support for aliyun oss #1961
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
Conversation
|
Hi @nanaya-tachibana ! Thanks for submitting your PR! Looks pretty good! 🙂
Thanks, |
098be92 to
a8031ca
Compare
I found an old oss emulate and made it compatible with the new oss python SDK. Start a container running an oss emulator. I also added these processes to travis and appveyor setting files. It was my first time using these continuous integration services. I hope I did correctly. |
efiop
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Please see a few more comments down below.
tests/func/test_data_cloud.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed? 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it safe to remove cli test? I commented this test because the gc cli test failed after I removed the codes in cache.py.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GC cli test? Google Cloud? Or garbage collector?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And what was the error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Traceback (most recent call last):
File "/Users/nanaya/Projects/dvc/dvc/main.py", line 38, in main
ret = cmd.run_cmd()
File "/Users/nanaya/Projects/dvc/dvc/command/base.py", line 60, in run_cmd
return self.run()
File "/Users/nanaya/Projects/dvc/dvc/command/gc.py", line 47, in run
repos=self.args.repos,
File "/Users/nanaya/Projects/dvc/dvc/repo/gc.py", line 113, in gc
_do_gc("remote", self.cloud._get_cloud(remote, "gc -c").gc, clist)
File "/Users/nanaya/Projects/dvc/dvc/repo/gc.py", line 54, in _do_gc
removed = func(clist)
File "/Users/nanaya/Projects/dvc/dvc/remote/base.py", line 489, in gc
used |= {info[self.PARAM_CHECKSUM] for info in cinfos[self.scheme]}
KeyError: 'oss'
cinfos is a dict which only has keys for 'local', 's3', 'gs', 'hdfs', 'ssh' and 'azure'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nanaya-tachibana Thanks! Looks like a bug. Does replacing cinfos[self.scheme] with cinfos.get(self.scheme, []) help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nanaya-tachibana Thanks! Looks like a bug. Does replacing
cinfos[self.scheme]withcinfos.get(self.scheme, [])help?
That can fix it. Should I add another commit to fix it and keep the command line tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nanaya-tachibana If you would be so kind 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, decided to help out a little bit 🙂
|
@nanaya-tachibana Btw, please rebase your patch on top of master. |
|
@nanaya-tachibana looks like you didn't rebase correctly. Try something like: |
Usage: $ dvc remote add myremote oss://my-bucket.endpoint/path Set key id and key secret using modify command $ dvc remote modify myremote oss_key_id my-key-id $ dvc remote modify myremote oss_key_secret my-key-secret or environment variables $ export OSS_ACCESS_KEY_ID="my-key-id" $ export OSS_ACCESS_KEY_SECRET="my-key-secret" Ref: oss python SDK: https://www.alibabacloud.com/help/doc-detail/32026.htm
Start a container running an oss emulator. $ git clone https://github.com/nanaya-tachibana/oss-emulator.git $ docker image build -t oss:1.0 oss-emulator $ docker run --detach -p 8880:8880 --name oss-emulator oss:1.0 Setup environment variables. $ export OSS_BUCKET='my-bucket' $ export OSS_ENDPOINT='localhost:8880' $ export OSS_ACCESS_KEY_ID='AccessKeyID' $ export OSS_ACCESS_KEY_SECRET='AccessKeySecret'
which gives read access to public read bucket and public bucket.
…able value. Usage: $ dvc remote add myremote oss://my-bucket/path Set key id, key secret and endpoint using modify command $ dvc remote modify myremote oss_key_id my-key-id $ dvc remote modify myremote oss_key_secret my-key-secret $ dvc remote modify myremote oss_endpoint endpoint or environment variables $ export OSS_ACCESS_KEY_ID="my-key-id" $ export OSS_ACCESS_KEY_SECRET="my-key-secret" $ export OSS_ENDPOINT="endpoint"
Signed-off-by: Ruslan Kuprieiev <ruslan@iterative.ai>
Signed-off-by: Ruslan Kuprieiev <ruslan@iterative.ai>
Just to keep things in-house. Signed-off-by: Ruslan Kuprieiev <ruslan@iterative.ai>
|
@nanaya-tachibana Truly outstanding work! 🥇 It is a pleasure working with you! Thank you so much! 🙂 |
|
|
||
| class RemoteOSS(RemoteBase): | ||
| """ | ||
| oss2 document: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😍
|
Hi @nanaya-tachibana ! Thanks again for your contribution! We would love to have a chance to meet you :) Would you be willing to have a hangouts call with us some time? If so, please ping me through email(in my profile) or on discord https://dvc.org/chat (I'm 'ruslan' there), so we could arrange something 🙂 Thanks, |
|
there's no docs about adding an oss remote, could you please add the corresponding instructions in the docs? thx |
#1954
Have you followed the guidelines in our
Contributing document?
Does your PR affect documented changes or does it add new functionality
that should be documented? If yes, have you created a PR for
dvc.org documenting it or at
least opened an issue for it? If so, please add a link to it. link