Skip to content

Conversation

@dtrifiro
Copy link
Contributor

@dtrifiro dtrifiro commented Feb 12, 2022

jobs kwarg was not being passed on properly

Fixes #7319

the `jobs` kwarg was not being passed down properly,
resulting in it being ignored in some cases (push/pull/gc)
@dtrifiro dtrifiro force-pushed the fix/7319-jobs-count-ignored branch from 74918b9 to dfcb7d2 Compare February 15, 2022 11:49
@dtrifiro dtrifiro marked this pull request as ready for review February 15, 2022 11:51
@dtrifiro dtrifiro requested a review from a team as a code owner February 15, 2022 11:51
@dtrifiro dtrifiro requested a review from pared February 15, 2022 11:51

odb = self.cloud.get_remote_odb(remote, "gc -c")
removed = ogc(odb, used_obj_ids)
removed = ogc(odb, used_obj_ids, jobs=jobs)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like we were ignoring job count in gc too

Copy link
Collaborator

@skshetry skshetry Feb 15, 2022

Choose a reason for hiding this comment

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

We have an issue for that: #5961.

Copy link
Contributor

@daavoo daavoo Feb 15, 2022

Choose a reason for hiding this comment

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

Should this P.R. close #5961 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm looking into how easy it would be to add basic multithreading to gc. For now, it only uses multithreading when listing objects from the odb (odb.all()) using _list_hashes_traverse.

I think it should #5961 should be closed in another PR.


def test_cloud_cli(tmp_dir, dvc, remote):
args = ["-v", "-j", "2"]
def test_cloud_cli(tmp_dir, dvc, remote, mocker):
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is getting crazier and crazier :) We'll get rid of it when we have lower level testing in dvc-data/objects.

Copy link
Contributor

@efiop efiop left a comment

Choose a reason for hiding this comment

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

Thank you, @dtrifiro ! 🙏

@efiop efiop merged commit 220c633 into treeverse:main Feb 18, 2022
@efiop efiop added the bugfix fixes bug label Feb 18, 2022
iesahin pushed a commit to treeverse/dvc.org that referenced this pull request Mar 3, 2022
Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Co-authored-by: restyled-io[bot] <32688539+restyled-io[bot]@users.noreply.github.com>


As [2.9.5](https://github.com/iterative/dvc/releases/tag/2.9.5) includes treeverse/dvc#7373, we can merge this now.
iesahin pushed a commit to treeverse/dvc.org that referenced this pull request Apr 11, 2022
Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Jorge Orpinel <jorgeorpinel@users.noreply.github.com>
Co-authored-by: restyled-io[bot] <32688539+restyled-io[bot]@users.noreply.github.com>


As [2.9.5](https://github.com/iterative/dvc/releases/tag/2.9.5) includes treeverse/dvc#7373, we can merge this now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix fixes bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

push: jobs count -j ignored; jobs count is not 4 for SSH (reason for #6856)

5 participants