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

GS progress for push & pull #2809

Merged
merged 11 commits into from
Nov 21, 2019
Merged

GS progress for push & pull #2809

merged 11 commits into from
Nov 21, 2019

Conversation

casperdcl
Copy link
Contributor

@casperdcl casperdcl commented Nov 17, 2019

@casperdcl casperdcl added p3-nice-to-have It should be done this or next sprint ui user interface / interaction labels Nov 17, 2019
@casperdcl casperdcl requested a review from efiop November 17, 2019 23:01
@casperdcl casperdcl self-assigned this Nov 17, 2019
@casperdcl
Copy link
Contributor Author

casperdcl commented Nov 17, 2019

@efiop @xliiv this is the sort of thing I intended. The only reason I hadn't implement it before is I don't know how to test it (#1566 (comment))

@efiop
Copy link
Contributor

efiop commented Nov 17, 2019

@casperdcl Looks great! I've also had this implementation in mind. Btw, check your email 😉

@casperdcl
Copy link
Contributor Author

I'm suspicious that the CI build passes - is GS actually tested on Travis?

@efiop
Copy link
Contributor

efiop commented Nov 17, 2019

@casperdcl GS is not tested on regular PRs, only on master, as it uses some secrets for accessing our GS account. As I've noted in the comment under the issue, I'll try this out myself, because setting up GS from scratch will take longer than writing this PR 🙂

dvc/remote/gs.py Outdated Show resolved Hide resolved
dvc/remote/gs.py Outdated Show resolved Hide resolved
dvc/remote/gs.py Outdated Show resolved Hide resolved
@efiop
Copy link
Contributor

efiop commented Nov 19, 2019

@casperdcl Just a heads up: gs chunking test failed. Just need to update it with the new default chunk size in mind

dvc/remote/gs.py Outdated Show resolved Hide resolved
dvc/remote/gs.py Show resolved Hide resolved
dvc/remote/gs.py Outdated Show resolved Hide resolved
dvc/remote/gs.py Outdated Show resolved Hide resolved
dvc/remote/gs.py Outdated Show resolved Hide resolved
@casperdcl casperdcl added refactoring Factoring and re-factoring research labels Nov 19, 2019
@casperdcl
Copy link
Contributor Author

@efiop think this is ready for merge

@efiop efiop requested review from Suor, a user and pared November 20, 2019 00:02
@efiop
Copy link
Contributor

efiop commented Nov 20, 2019

@Suor Since you've left a review earlier, please take another look. Thank you 🙂

@Suor
Copy link
Contributor

Suor commented Nov 20, 2019

After progress discussion is resolved and if we are sure 100 MB chunks are not needed it will be good to merge.

@Suor Suor mentioned this pull request Nov 20, 2019
2 tasks
@casperdcl casperdcl mentioned this pull request Nov 21, 2019
@efiop efiop merged commit 7bc7a57 into iterative:master Nov 21, 2019
@efiop
Copy link
Contributor

efiop commented Nov 21, 2019

@casperdcl Thank you! 🙏

@casperdcl casperdcl deleted the gs-progress branch November 21, 2019 23:53
@casperdcl casperdcl mentioned this pull request Dec 1, 2019
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances DVC p3-nice-to-have It should be done this or next sprint refactoring Factoring and re-factoring research ui user interface / interaction
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gs: support progress callback
4 participants