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.

Sign up for GitHub

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

Jump to bottom

spec: Wrap public methods. #48

Merged
merged 1 commit into from
May 31, 2023
Merged

spec: Wrap public methods. #48

merged 1 commit into from
May 31, 2023

Conversation

daavoo
Copy link
Contributor

@daavoo daavoo commented May 31, 2023 •
edited
Loading

The wrapping of the private async methods was not working as expected.

In dvc-http, the wrapping is working because the implementation doesn't use sync_wrapper, but in adlfs we need to do it this way:

https://github.com/fsspec/adlfs/blob/71c068458390192a4753e7127ba9c244eec59697/adlfs/spec.py#L1619

For #50

@daavoo
spec: Wrap public methods.
b851120 
The wrapping of the private async methods was not working as expected.
@daavoo daavoo self-assigned this May 31, 2023
@daavoo daavoo requested a review from a team May 31, 2023 10:48
@github-actions
Copy link
Contributor 


------------------------------------------------------------------------ benchmark 'test_sharing_azure-add': 7 tests ------------------------------------------------------------------------
Name (time in s)                      Min               Max              Mean            StdDev            Median               IQR            Outliers     OPS            Rounds  Iterations
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_sharing_azure-add[2.11.0]     6.3873 (1.60)     6.3873 (1.60)     6.3873 (1.60)     0.0000 (1.0)      6.3873 (1.60)     0.0000 (1.0)           0;0  0.1566 (0.63)          1           1
test_sharing_azure-add[2.18.1]     5.5220 (1.38)     5.5220 (1.38)     5.5220 (1.38)     0.0000 (1.0)      5.5220 (1.38)     0.0000 (1.0)           0;0  0.1811 (0.72)          1           1
test_sharing_azure-add[2.39.0]     5.2671 (1.32)     5.2671 (1.32)     5.2671 (1.32)     0.0000 (1.0)      5.2671 (1.32)     0.0000 (1.0)           0;0  0.1899 (0.76)          1           1
test_sharing_azure-add[2.40.0]     5.5556 (1.39)     5.5556 (1.39)     5.5556 (1.39)     0.0000 (1.0)      5.5556 (1.39)     0.0000 (1.0)           0;0  0.1800 (0.72)          1           1
test_sharing_azure-add[2.41.1]     5.3533 (1.34)     5.3533 (1.34)     5.3533 (1.34)     0.0000 (1.0)      5.3533 (1.34)     0.0000 (1.0)           0;0  0.1868 (0.75)          1           1
test_sharing_azure-add[2.45.0]     3.9968 (1.0)      3.9968 (1.0)      3.9968 (1.0)      0.0000 (1.0)      3.9968 (1.0)      0.0000 (1.0)           0;0  0.2502 (1.0)           1           1
test_sharing_azure-add[main]       4.0714 (1.02)     4.0714 (1.02)     4.0714 (1.02)     0.0000 (1.0)      4.0714 (1.02)     0.0000 (1.0)           0;0  0.2456 (0.98)          1           1
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------ benchmark 'test_sharing_azure-add-noop': 7 tests ------------------------------------------------------------------------
Name (time in s)                           Min               Max              Mean            StdDev            Median               IQR            Outliers     OPS            Rounds  Iterations
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_sharing_azure-add-noop[2.11.0]     2.4063 (1.0)      2.4063 (1.0)      2.4063 (1.0)      0.0000 (1.0)      2.4063 (1.0)      0.0000 (1.0)           0;0  0.4156 (1.0)           1           1
test_sharing_azure-add-noop[2.18.1]     2.5814 (1.07)     2.5814 (1.07)     2.5814 (1.07)     0.0000 (1.0)      2.5814 (1.07)     0.0000 (1.0)           0;0  0.3874 (0.93)          1           1
test_sharing_azure-add-noop[2.39.0]     2.4875 (1.03)     2.4875 (1.03)     2.4875 (1.03)     0.0000 (1.0)      2.4875 (1.03)     0.0000 (1.0)           0;0  0.4020 (0.97)          1           1
test_sharing_azure-add-noop[2.40.0]     2.4623 (1.02)     2.4623 (1.02)     2.4623 (1.02)     0.0000 (1.0)      2.4623 (1.02)     0.0000 (1.0)           0;0  0.4061 (0.98)          1           1
test_sharing_azure-add-noop[2.41.1]     2.4570 (1.02)     2.4570 (1.02)     2.4570 (1.02)     0.0000 (1.0)      2.4570 (1.02)     0.0000 (1.0)           0;0  0.4070 (0.98)          1           1
test_sharing_azure-add-noop[2.45.0]     2.5770 (1.07)     2.5770 (1.07)     2.5770 (1.07)     0.0000 (1.0)      2.5770 (1.07)     0.0000 (1.0)           0;0  0.3881 (0.93)          1           1
test_sharing_azure-add-noop[main]       2.7401 (1.14)     2.7401 (1.14)     2.7401 (1.14)     0.0000 (1.0)      2.7401 (1.14)     0.0000 (1.0)           0;0  0.3649 (0.88)          1           1
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

---------------------------------------------------------------------------- benchmark 'test_sharing_azure-checkout-noop': 7 tests ----------------------------------------------------------------------------
Name (time in ms)                                 Min                 Max                Mean            StdDev              Median               IQR            Outliers     OPS            Rounds  Iterations
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_sharing_azure-checkout-noop[2.11.0]     499.3763 (1.0)      499.3763 (1.0)      499.3763 (1.0)      0.0000 (1.0)      499.3763 (1.0)      0.0000 (1.0)           0;0  2.0025 (1.0)           1           1
test_sharing_azure-checkout-noop[2.18.1]     554.5100 (1.11)     554.5100 (1.11)     554.5100 (1.11)     0.0000 (1.0)      554.5100 (1.11)     0.0000 (1.0)           0;0  1.8034 (0.90)          1           1
test_sharing_azure-checkout-noop[2.39.0]     577.5458 (1.16)     577.5458 (1.16)     577.5458 (1.16)     0.0000 (1.0)      577.5458 (1.16)     0.0000 (1.0)           0;0  1.7315 (0.86)          1           1
test_sharing_azure-checkout-noop[2.40.0]     595.9514 (1.19)     595.9514 (1.19)     595.9514 (1.19)     0.0000 (1.0)      595.9514 (1.19)     0.0000 (1.0)           0;0  1.6780 (0.84)          1           1
test_sharing_azure-checkout-noop[2.41.1]     601.7414 (1.20)     601.7414 (1.20)     601.7414 (1.20)     0.0000 (1.0)      601.7414 (1.20)     0.0000 (1.0)           0;0  1.6618 (0.83)          1           1
test_sharing_azure-checkout-noop[2.45.0]     627.1020 (1.26)     627.1020 (1.26)     627.1020 (1.26)     0.0000 (1.0)      627.1020 (1.26)     0.0000 (1.0)           0;0  1.5946 (0.80)          1           1
test_sharing_azure-checkout-noop[main]       696.0165 (1.39)     696.0165 (1.39)     696.0165 (1.39)     0.0000 (1.0)      696.0165 (1.39)     0.0000 (1.0)           0;0  1.4367 (0.72)          1           1
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------ benchmark 'test_sharing_azure-pull': 7 tests ------------------------------------------------------------------------
Name (time in s)                       Min               Max              Mean            StdDev            Median               IQR            Outliers     OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_sharing_azure-pull[2.11.0]     4.7453 (1.12)     4.7453 (1.12)     4.7453 (1.12)     0.0000 (1.0)      4.7453 (1.12)     0.0000 (1.0)           0;0  0.2107 (0.89)          1           1
test_sharing_azure-pull[2.18.1]     4.7949 (1.13)     4.7949 (1.13)     4.7949 (1.13)     0.0000 (1.0)      4.7949 (1.13)     0.0000 (1.0)           0;0  0.2086 (0.88)          1           1
test_sharing_azure-pull[2.39.0]     4.9861 (1.18)     4.9861 (1.18)     4.9861 (1.18)     0.0000 (1.0)      4.9861 (1.18)     0.0000 (1.0)           0;0  0.2006 (0.85)          1           1
test_sharing_azure-pull[2.40.0]     5.0239 (1.19)     5.0239 (1.19)     5.0239 (1.19)     0.0000 (1.0)      5.0239 (1.19)     0.0000 (1.0)           0;0  0.1990 (0.84)          1           1
test_sharing_azure-pull[2.41.1]     5.1400 (1.22)     5.1400 (1.22)     5.1400 (1.22)     0.0000 (1.0)      5.1400 (1.22)     0.0000 (1.0)           0;0  0.1946 (0.82)          1           1
test_sharing_azure-pull[2.45.0]     4.2627 (1.01)     4.2627 (1.01)     4.2627 (1.01)     0.0000 (1.0)      4.2627 (1.01)     0.0000 (1.0)           0;0  0.2346 (0.99)          1           1
test_sharing_azure-pull[main]       4.2261 (1.0)      4.2261 (1.0)      4.2261 (1.0)      0.0000 (1.0)      4.2261 (1.0)      0.0000 (1.0)           0;0  0.2366 (1.0)           1           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

---------------------------------------------------------------------------- benchmark 'test_sharing_azure-pull-noop': 7 tests ----------------------------------------------------------------------------
Name (time in ms)                             Min                 Max                Mean            StdDev              Median               IQR            Outliers     OPS            Rounds  Iterations
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_sharing_azure-pull-noop[2.11.0]     541.7172 (1.0)      541.7172 (1.0)      541.7172 (1.0)      0.0000 (1.0)      541.7172 (1.0)      0.0000 (1.0)           0;0  1.8460 (1.0)           1           1
test_sharing_azure-pull-noop[2.18.1]     587.2311 (1.08)     587.2311 (1.08)     587.2311 (1.08)     0.0000 (1.0)      587.2311 (1.08)     0.0000 (1.0)           0;0  1.7029 (0.92)          1           1
test_sharing_azure-pull-noop[2.39.0]     616.8964 (1.14)     616.8964 (1.14)     616.8964 (1.14)     0.0000 (1.0)      616.8964 (1.14)     0.0000 (1.0)           0;0  1.6210 (0.88)          1           1
test_sharing_azure-pull-noop[2.40.0]     627.5449 (1.16)     627.5449 (1.16)     627.5449 (1.16)     0.0000 (1.0)      627.5449 (1.16)     0.0000 (1.0)           0;0  1.5935 (0.86)          1           1
test_sharing_azure-pull-noop[2.41.1]     639.9826 (1.18)     639.9826 (1.18)     639.9826 (1.18)     0.0000 (1.0)      639.9826 (1.18)     0.0000 (1.0)           0;0  1.5625 (0.85)          1           1
test_sharing_azure-pull-noop[2.45.0]     680.1020 (1.26)     680.1020 (1.26)     680.1020 (1.26)     0.0000 (1.0)      680.1020 (1.26)     0.0000 (1.0)           0;0  1.4704 (0.80)          1           1
test_sharing_azure-pull-noop[main]       753.9970 (1.39)     753.9970 (1.39)     753.9970 (1.39)     0.0000 (1.0)      753.9970 (1.39)     0.0000 (1.0)           0;0  1.3263 (0.72)          1           1
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------ benchmark 'test_sharing_azure-push': 7 tests ------------------------------------------------------------------------
Name (time in s)                       Min               Max              Mean            StdDev            Median               IQR            Outliers     OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_sharing_azure-push[2.11.0]     2.6265 (1.25)     2.6265 (1.25)     2.6265 (1.25)     0.0000 (1.0)      2.6265 (1.25)     0.0000 (1.0)           0;0  0.3807 (0.80)          1           1
test_sharing_azure-push[2.18.1]     2.5119 (1.19)     2.5119 (1.19)     2.5119 (1.19)     0.0000 (1.0)      2.5119 (1.19)     0.0000 (1.0)           0;0  0.3981 (0.84)          1           1
test_sharing_azure-push[2.39.0]     2.9103 (1.38)     2.9103 (1.38)     2.9103 (1.38)     0.0000 (1.0)      2.9103 (1.38)     0.0000 (1.0)           0;0  0.3436 (0.72)          1           1
test_sharing_azure-push[2.40.0]     2.7520 (1.31)     2.7520 (1.31)     2.7520 (1.31)     0.0000 (1.0)      2.7520 (1.31)     0.0000 (1.0)           0;0  0.3634 (0.77)          1           1
test_sharing_azure-push[2.41.1]     2.7797 (1.32)     2.7797 (1.32)     2.7797 (1.32)     0.0000 (1.0)      2.7797 (1.32)     0.0000 (1.0)           0;0  0.3597 (0.76)          1           1
test_sharing_azure-push[2.45.0]     2.1085 (1.0)      2.1085 (1.0)      2.1085 (1.0)      0.0000 (1.0)      2.1085 (1.0)      0.0000 (1.0)           0;0  0.4743 (1.0)           1           1
test_sharing_azure-push[main]       2.2189 (1.05)     2.2189 (1.05)     2.2189 (1.05)     0.0000 (1.0)      2.2189 (1.05)     0.0000 (1.0)           0;0  0.4507 (0.95)          1           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

---------------------------------------------------------------------------- benchmark 'test_sharing_azure-push-noop': 7 tests ----------------------------------------------------------------------------
Name (time in ms)                             Min                 Max                Mean            StdDev              Median               IQR            Outliers     OPS            Rounds  Iterations
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_sharing_azure-push-noop[2.11.0]     447.9777 (1.01)     447.9777 (1.01)     447.9777 (1.01)     0.0000 (1.0)      447.9777 (1.01)     0.0000 (1.0)           0;0  2.2323 (0.99)          1           1
test_sharing_azure-push-noop[2.18.1]     443.6344 (1.0)      443.6344 (1.0)      443.6344 (1.0)      0.0000 (1.0)      443.6344 (1.0)      0.0000 (1.0)           0;0  2.2541 (1.0)           1           1
test_sharing_azure-push-noop[2.39.0]     496.3543 (1.12)     496.3543 (1.12)     496.3543 (1.12)     0.0000 (1.0)      496.3543 (1.12)     0.0000 (1.0)           0;0  2.0147 (0.89)          1           1
test_sharing_azure-push-noop[2.40.0]     478.0005 (1.08)     478.0005 (1.08)     478.0005 (1.08)     0.0000 (1.0)      478.0005 (1.08)     0.0000 (1.0)           0;0  2.0920 (0.93)          1           1
test_sharing_azure-push-noop[2.41.1]     499.2190 (1.13)     499.2190 (1.13)     499.2190 (1.13)     0.0000 (1.0)      499.2190 (1.13)     0.0000 (1.0)           0;0  2.0031 (0.89)          1           1
test_sharing_azure-push-noop[2.45.0]     510.7638 (1.15)     510.7638 (1.15)     510.7638 (1.15)     0.0000 (1.0)      510.7638 (1.15)     0.0000 (1.0)           0;0  1.9579 (0.87)          1           1
test_sharing_azure-push-noop[main]       589.9570 (1.33)     589.9570 (1.33)     589.9570 (1.33)     0.0000 (1.0)      589.9570 (1.33)     0.0000 (1.0)           0;0  1.6950 (0.75)          1           1
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Legend:
  Outliers: 1 Standard Deviation from Mean; 1.5 IQR (InterQuartile Range) from 1st Quartile and 3rd Quartile.
  OPS: Operations Per Second, computed as 1 / Mean

@daavoo
Copy link
Contributor Author

daavoo commented May 31, 2023

I manually tested this (we cant test cloud versioning on azure for now)

@daavoo daavoo merged commit 0390ddc into main May 31, 2023
@daavoo daavoo deleted the fix-wrapping branch May 31, 2023 11:01
daavoo added a commit to iterative/dvc that referenced this pull request May 31, 2023
@daavoo
deps: bumps dvc-azure>=2.21.2
ab48e10 
Closes #9506 via iterative/dvc-azure#48
@daavoo daavoo mentioned this pull request May 31, 2023
Merged
daavoo added a commit to iterative/dvc that referenced this pull request May 31, 2023
@daavoo
deps: bumps dvc-azure>=2.21.2
a1b891c 
Closes #9506 via iterative/dvc-azure#48
mergify bot pushed a commit to iterative/dvc that referenced this pull request May 31, 2023
@daavoo @mergify
deps: bumps dvc-azure>=2.21.2
4b7f818 
Closes #9506 via iterative/dvc-azure#48

(cherry picked from commit a1b891c)
daavoo added a commit to iterative/dvc that referenced this pull request May 31, 2023
@daavoo
deps: bumps dvc-azure>=2.21.2
6a5fa99 
Closes #9506 via iterative/dvc-azure#48

(cherry picked from commit a1b891c)
pmrowla
pmrowla reviewed Jun 21, 2023
View reviewed changes
dvc_azure/spec.py
Comment on lines +7 to +9
def put_file(self, *args, **kwargs) -> None:
kwargs["overwrite"] = True
return await super()._put_file(*args, **kwargs)
return super().put_file(*args, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

This breaks the case where we use the async coroutines directly in batched file operations. We need to be setting overwrite in both the sync put_file and async _put_file versions of the method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pmrowla pmrowla pmrowla left review comments

Assignees

@daavoo daavoo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@daavoo @pmrowla