[Data] Add read API for Delta sharing tables #46072

brucebismarck · 2024-06-16T04:58:13Z

Why are these changes needed?

Databricks prefers to use deltasharing rather than execution statement SQL to share data to external.
Using execution statement SQL (current databricks_uc_datasource) will have a 100GB limit, data will be truncated.
https://github.com/ray-project/ray/blob/master/python/ray/data/datasource/databricks_uc_datasource.py#L99-L103

This is the design/decision from databricks. However, using delta sharing does not have this limit.
I have tested locally to pull 180+ GB data from databricks to ray data.

With this, transferring data from one of the most famous/trusted data management system Databricks to Ray will be much easier. Otherwise, the best solution is use a spark job dump data to cloud drive (s3) and then read from there.

Related issue number

Checks

I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.io/en/master/.
- I've added any new APIs to the API Reference. For example, if I added a
  method in Tune, I've added it in doc/source/tune/api/ under the
  corresponding .rst file.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested :(

c21

Thank you @brucebismarck for the contribution! Looks solid. Having some comments.

python/ray/data/read_api.py

c21 · 2024-06-17T17:27:54Z

python/ray/data/read_api.py

+    version: Optional[int] = None,
+    timestamp: Optional[str] = None,
+    jsonPredicateHints: Optional[str] = None,
+    parallelism: int = -1,


parallelism is deprecated, we can remove it.

c21 · 2024-06-17T17:28:29Z

python/ray/data/read_api.py

+
+    This function reads data from a Delta Sharing table specified by the URL.
+    It supports various options such as limiting the number of rows, specifying
+    a version or timestamp, and configuring parallelism and concurrency.


configuring parallelism and concurrency -> configuring concurrency.

python/ray/data/read_api.py

c21 · 2024-06-17T17:29:32Z

python/ray/data/read_api.py

+                        ml_adhoc_data_sharing.ralph_user_pairs_with_seq_features_310d_p01_train",
+                limit=100000,
+                version=1,
+            ).materialize()


.materialize() can be removed.

c21 · 2024-06-17T17:38:31Z

python/ray/data/read_api.py

+        setup_delta_sharing_connections,
+    )
+
+    table, rest_client = setup_delta_sharing_connections(url)


can this be called instead inside DeltaSharingDatasource.__init__?

c21 · 2024-06-17T17:39:01Z

python/ray/data/datasource/delta_sharing_datasource.py

+    :param url: a url under the format "<profile>#<share>.<schema>.<table>"
+    :
+    """
+    from delta_sharing.protocol import DeltaSharingProfile, Table


ditto: change to _check_import.

python/ray/data/datasource/delta_sharing_datasource.py

c21 · 2024-06-17T17:40:13Z

python/ray/data/tests/test_deltasharing.py

@@ -0,0 +1,133 @@
+import json


nit: for file name, let's keep consistent with test_delta_sharing.py

c21 · 2024-06-17T17:41:08Z

python/ray/data/tests/test_deltasharing.py

+        self.assertEqual(table.share, "share")
+        self.assertEqual(table.schema, "schema")
+        self.assertIsInstance(rest_client, DataSharingRestClient)
+


is it possible if we can test ray.data.read_delta_sharing_tables as well?

brucebismarck · 2024-06-18T05:55:11Z

Couple of questions:

in my test code, I got this error locally.
FAILED python/ray/data/tests/test_delta_sharing.py::TestReadDeltaSharingTables::test_read_delta_sharing_tables - AttributeError: module 'ray.data' has no attribute 'read_delta_sharing_tables'
How do you make this importable?
I got this error when I run git commit
flake8...................................................................Failed

hook id: flake8
exit code: 1

Traceback (most recent call last):
File "/Users/wenyueliu/.cache/pre-commit/repo0ol9jsjm/py_env-python3.12/bin/flake8", line 8, in
sys.exit(main())
^^^^^^
File "/Users/wenyueliu/.cache/pre-commit/repo0ol9jsjm/py_env-python3.12/lib/python3.12/site-packages/flake8/main/cli.py", line 22, in main
app.run(argv)
File "/Users/wenyueliu/.cache/pre-commit/repo0ol9jsjm/py_env-python3.12/lib/python3.12/site-packages/flake8/main/application.py", line 363, in run
self._run(argv)
File "/Users/wenyueliu/.cache/pre-commit/repo0ol9jsjm/py_env-python3.12/lib/python3.12/site-packages/flake8/main/application.py", line 350, in _run
self.initialize(argv)
File "/Users/wenyueliu/.cache/pre-commit/repo0ol9jsjm/py_env-python3.12/lib/python3.12/site-packages/flake8/main/application.py", line 330, in initialize
self.find_plugins(config_finder)
File "/Users/wenyueliu/.cache/pre-commit/repo0ol9jsjm/py_env-python3.12/lib/python3.12/site-packages/flake8/main/application.py", line 153, in find_plugins
self.check_plugins = plugin_manager.Checkers(local_plugins.extension)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/wenyueliu/.cache/pre-commit/repo0ol9jsjm/py_env-python3.12/lib/python3.12/site-packages/flake8/plugins/manager.py", line 356, in init
self.manager = PluginManager(
^^^^^^^^^^^^^^
File "/Users/wenyueliu/.cache/pre-commit/repo0ol9jsjm/py_env-python3.12/lib/python3.12/site-packages/flake8/plugins/manager.py", line 238, in init
self._load_entrypoint_plugins()
File "/Users/wenyueliu/.cache/pre-commit/repo0ol9jsjm/py_env-python3.12/lib/python3.12/site-packages/flake8/plugins/manager.py", line 254, in _load_entrypoint_plugins
eps = importlib_metadata.entry_points().get(self.namespace, ())
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'EntryPoints' object has no attribute 'get'
Looks like flake related package error?

precommit failed --> UT lint check failed. How can i fix it?

Tested this in my dataset and it works.

c21 · 2024-06-18T17:55:33Z

Fix lint error:

[2024-06-18T06:03:23Z] python/ray/data/tests/test_delta_sharing.py:6:1: F401 'delta_sharing.protocol.DeltaSharingProfile' imported but unused
--
  | [2024-06-18T06:03:23Z] python/ray/data/tests/test_delta_sharing.py:46:89: E501 line too long (109 > 88 characters)
  | [2024-06-18T06:03:23Z] python/ray/data/tests/test_delta_sharing.py:69:89: E501 line too long (109 > 88 characters)
  | [2024-06-18T06:03:23Z] python/ray/data/tests/test_delta_sharing.py:89:89: E501 line too long (109 > 88 characters)

You can also run the followed script to fix format:

ray % ./scripts/format.sh

Fix unit test error:

The error is:

ModuleNotFoundError: No module named 'delta_sharing'

Ray CI does not install delta_sharing automatically. You can add the package in python/requirements/ml/data-test-requirements.txt. See #45106 for an example.

Add the API into __init__.py files and doc file

python/ray/data/__init__.py, python/ray/data/datasource/__init__.py and doc/source/data/api/input_output.rst. You can follow example in #45106 .

checkin lint update read_api update read_api add docstring, ut etc. updates revert precommit-config checkin-last address comments update tests update tests Signed-off-by: Wenyue Liu <[email protected]>

Signed-off-by: Wenyue Liu <[email protected]>

scottjlee · 2024-06-28T21:57:38Z

Merging in master to attempt to resolve CI failures

anyscalesam · 2024-06-29T01:52:20Z

looks like there are merge conflicts @brucebismarck can you review?

brucebismarck · 2024-06-29T07:51:13Z

looks like there are merge conflicts @brucebismarck can you review?

@anyscalesam
Sorry I'm AFK till July 6th, but resolved this through UI.

raulchen · 2024-07-10T23:14:35Z

python/ray/data/tests/test_delta_sharing.py

+if __name__ == "__main__":
+    unittest.main()


We usually use pytest as the main function.
Not sure if unittest also works well.
But let's keep it consistent with other files.

Suggested change

if __name__ == "__main__":

unittest.main()

if __name__ == "__main__":

import sys

sys.exit(pytest.main(["-v", __file__]))

raulchen · 2024-07-10T23:20:39Z

python/ray/data/datasource/delta_sharing_datasource.py

+    def __init__(
+        self,
+        url: str,
+        jsonPredicateHints: Optional[str] = None,


noticed some variables using the camel case naming convention. Can you update them to use snake_case?
We follow google's python coding style guide.

Signed-off-by: Wenyue Liu <[email protected]>

…ad task part Signed-off-by: Wenyue Liu <[email protected]>

brucebismarck requested review from ericl, scv119, c21, amogkam, scottjlee, bveeramani, raulchen, stephanie-wang and omatthew98 as code owners June 16, 2024 04:58

brucebismarck force-pushed the DeltaSharing-to-raydata branch from 56fe0f1 to 8019e9b Compare June 16, 2024 05:10

brucebismarck changed the title ~~Delta sharing to raydata~~ [Data] Delta sharing to raydata Jun 16, 2024

c21 reviewed Jun 17, 2024

View reviewed changes

c21 added the go add ONLY when ready to merge, run all tests label Jun 17, 2024

anyscalesam added @external-author-action-required Alternate tag for PRs where the author doesn't have labeling permission. data Ray Data-related issues labels Jun 17, 2024

brucebismarck force-pushed the DeltaSharing-to-raydata branch from e5a16dc to fe364af Compare June 18, 2024 05:35

scottjlee self-assigned this Jun 18, 2024

brucebismarck and others added 4 commits June 23, 2024 16:04

add DeltaSharing data source

7d385f5

checkin lint update read_api update read_api add docstring, ut etc. updates revert precommit-config checkin-last address comments update tests update tests Signed-off-by: Wenyue Liu <[email protected]>

forgot to add self

3c8e3d7

Signed-off-by: Wenyue Liu <[email protected]>

address comments

fa1a708

Signed-off-by: Wenyue Liu <[email protected]>

doc

094d8ff

Signed-off-by: Wenyue Liu <[email protected]>

brucebismarck force-pushed the DeltaSharing-to-raydata branch from c077dc8 to 094d8ff Compare June 23, 2024 23:04

update tests, however there is an error

a7edaca

Signed-off-by: Wenyue Liu <[email protected]>

brucebismarck requested a review from c21 June 25, 2024 00:08

anyscalesam assigned c21 Jun 29, 2024

Merge branch 'master' into DeltaSharing-to-raydata

8b6ddfb

brucebismarck force-pushed the DeltaSharing-to-raydata branch from 460b552 to 8b6ddfb Compare July 8, 2024 22:18

refactor

76f306f

raulchen approved these changes Jul 10, 2024

View reviewed changes

raulchen changed the title ~~[Data] Delta sharing to raydata~~ [Data] Add read API for Delta sharing tables Jul 10, 2024

brucebismarck added 3 commits July 10, 2024 16:38

address naming convertion comments

f353dd0

Merge branch 'master' into DeltaSharing-to-raydata

8b0ec34

lint

e02de9a

Signed-off-by: Wenyue Liu <[email protected]>

brucebismarck force-pushed the DeltaSharing-to-raydata branch from fc8fb34 to e02de9a Compare July 10, 2024 23:49

brucebismarck and others added 5 commits July 11, 2024 15:41

update typo

12adcc1

Signed-off-by: Wenyue Liu <[email protected]>

Merge branch 'master' into DeltaSharing-to-raydata

bbc8935

remove one unit test as it consistently causing error from pickling

52d75ff

Signed-off-by: Wenyue Liu <[email protected]>

Merge branch 'master' into DeltaSharing-to-raydata

6d24c5c

add back

205c68a

Signed-off-by: Wenyue Liu <[email protected]>

raulchen self-assigned this Jul 12, 2024

anyscalesam added the triage Needs triage (eg: priority, bug/not-bug, and owning component) label Jul 16, 2024

brucebismarck added 2 commits July 16, 2024 16:58

Merge branch 'master' into DeltaSharing-to-raydata

c0ee53e

update unit tests and move delta sharing calling code from init to re…

a78eaf0

…ad task part Signed-off-by: Wenyue Liu <[email protected]>

raulchen merged commit 593d04a into ray-project:master Jul 17, 2024
4 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Data] Add read API for Delta sharing tables #46072

[Data] Add read API for Delta sharing tables #46072

brucebismarck commented Jun 16, 2024 •

edited

Loading

c21 left a comment

c21 Jun 17, 2024

c21 Jun 17, 2024

c21 Jun 17, 2024

c21 Jun 17, 2024

c21 Jun 17, 2024

c21 Jun 17, 2024

c21 Jun 17, 2024

brucebismarck commented Jun 18, 2024 •

edited

Loading

c21 commented Jun 18, 2024

scottjlee commented Jun 28, 2024

anyscalesam commented Jun 29, 2024

brucebismarck commented Jun 29, 2024 •

edited

Loading

raulchen Jul 10, 2024

raulchen Jul 10, 2024

[Data] Add read API for Delta sharing tables #46072

[Data] Add read API for Delta sharing tables #46072

Conversation

brucebismarck commented Jun 16, 2024 • edited Loading

Why are these changes needed?

Related issue number

Checks

c21 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brucebismarck commented Jun 18, 2024 • edited Loading

c21 commented Jun 18, 2024

scottjlee commented Jun 28, 2024

anyscalesam commented Jun 29, 2024

brucebismarck commented Jun 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brucebismarck commented Jun 16, 2024 •

edited

Loading

brucebismarck commented Jun 18, 2024 •

edited

Loading

brucebismarck commented Jun 29, 2024 •

edited

Loading