[SCHEMATIC-183] Use paths from file view for manifest generation #1529

GiaJordan · 2024-10-30T20:35:32Z

Resolves SCHEMATIC-183 and switches to using the path column from synapse file views for file paths when generating manifests and getting files in a dataset.

Tests are provided in a couple of formats per conversations with @thomasyu888

thomasyu888 · 2024-11-20T02:54:41Z

schematic/store/synapse.py

-            synapse_id=datasetId, syn=self.syn, download_file=False
-        )
+        # Identify all folders nested under the dataset folder
+        folders = synapseutils.walk(self.syn, datasetId, includeTypes=["folder"])


Just a note: Analysis like this can be done outside of the code. I took a look at one of the NF projects (JHU biobank) and I didn't realize projects have such deep nesting so to be extreme

62 folders within the folder will take around 11 seconds for both solutions (this is an actual folder count)

9000 folders however... I didn't finish the walk solution but it took ~20 seconds for the entityview solution. (This is hypothetical, I'm not sure what the upper bound is - we can find out in snowflake but my power is out) I can show you the query to find out this upper bound if youd like.

One other thought if you had the fileview, is using the path and do something like this ..

select * from .... where path like 'myproject/myfolder/path/%'

That should get you all the files under a folder

tests/integration/test_commands.py

GiaJordan · 2024-11-20T18:21:52Z

@BryanFauble I've made some changes and addressed the comments, if you want to review it again it's ready now. The tests that failed previously are passing locally now so there should only be minimal changes if any going forward

GiaJordan · 2024-11-21T16:41:21Z

@BryanFauble have you seen an error like the ones from the latest run of the integration tests action?

--- Logging error ---
Traceback (most recent call last):
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connection.py", line 199, in _new_conn
    sock = connection.create_connection(
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/util/connection.py", line 60, in create_connection
    for res in socket.getaddrinfo(host, port, family, socket.SOCK_STREAM):
  File "/opt/hostedtoolcache/Python/3.10.15/x64/lib/python3.10/socket.py", line 967, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
socket.gaierror: [Errno -2] Name or service not known

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connectionpool.py", line 789, in urlopen
    response = self._make_request(
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connectionpool.py", line 490, in _make_request
    raise new_e
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connectionpool.py", line 466, in _make_request
    self._validate_conn(conn)
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connectionpool.py", line 1095, in _validate_conn
    conn.connect()
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connection.py", line 693, in connect
    self.sock = sock = self._new_conn()
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connection.py", line 206, in _new_conn
    raise NameResolutionError(self.host, self, e) from e
urllib3.exceptions.NameResolutionError: <urllib3.connection.HTTPSConnection object at 0x7f1ccf6fc8e0>: Failed to resolve 'dev.sagedpe.org' ([Errno -2] Name or service not known)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/requests/adapters.py", line 667, in send
    resp = conn.urlopen(
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connectionpool.py", line 843, in urlopen
    retries = retries.increment(
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/util/retry.py", line 519, in increment
    raise MaxRetryError(_pool, url, reason) from reason  # type: ignore[arg-type]
urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='dev.sagedpe.org', port=[44](https://github.com/Sage-Bionetworks/schematic/actions/runs/11942387272/job/33289323317?pr=1529#step:11:45)3): Max retries exceeded with url: /telemetry/v1/traces (Caused by NameResolutionError("<urllib3.connection.HTTPSConnection object at 0x7f1ccf6fc8e0>: Failed to resolve 'dev.sagedpe.org' ([Errno -2] Name or service not known)"))

During handling of the above exception, another exception occurred:

BryanFauble · 2024-11-21T17:00:43Z

@BryanFauble have you seen an error like the ones from the latest run of the integration tests action?

--- Logging error ---
Traceback (most recent call last):
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connection.py", line 199, in _new_conn
    sock = connection.create_connection(
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/util/connection.py", line 60, in create_connection
    for res in socket.getaddrinfo(host, port, family, socket.SOCK_STREAM):
  File "/opt/hostedtoolcache/Python/3.10.15/x64/lib/python3.10/socket.py", line 967, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
socket.gaierror: [Errno -2] Name or service not known

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connectionpool.py", line 789, in urlopen
    response = self._make_request(
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connectionpool.py", line 490, in _make_request
    raise new_e
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connectionpool.py", line 466, in _make_request
    self._validate_conn(conn)
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connectionpool.py", line 1095, in _validate_conn
    conn.connect()
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connection.py", line 693, in connect
    self.sock = sock = self._new_conn()
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connection.py", line 206, in _new_conn
    raise NameResolutionError(self.host, self, e) from e
urllib3.exceptions.NameResolutionError: <urllib3.connection.HTTPSConnection object at 0x7f1ccf6fc8e0>: Failed to resolve 'dev.sagedpe.org' ([Errno -2] Name or service not known)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/requests/adapters.py", line 667, in send
    resp = conn.urlopen(
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/connectionpool.py", line 843, in urlopen
    retries = retries.increment(
  File "/home/runner/work/schematic/schematic/.venv/lib/python3.10/site-packages/urllib3/util/retry.py", line 519, in increment
    raise MaxRetryError(_pool, url, reason) from reason  # type: ignore[arg-type]
urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='dev.sagedpe.org', port=[44](https://github.com/Sage-Bionetworks/schematic/actions/runs/11942387272/job/33289323317?pr=1529#step:11:45)3): Max retries exceeded with url: /telemetry/v1/traces (Caused by NameResolutionError("<urllib3.connection.HTTPSConnection object at 0x7f1ccf6fc8e0>: Failed to resolve 'dev.sagedpe.org' ([Errno -2] Name or service not known)"))

During handling of the above exception, another exception occurred:

Yeah I am already working to resolve this.

It's probably not the root cause of the test failure, but it's obfuscating what the real reason of the test failures are. I will re-run the GH action when I get this resolved.

GiaJordan · 2024-11-21T17:04:30Z

Yeah I am already working to resolve this.

It's probably not the root cause of the test failure, but it's obfuscating what the real reason of the test failures are. I will re-run the GH action when I get this resolved.

Okay, thanks for the heads up! @BryanFauble

thomasyu888

tests/test_api.py::TestSynapseStorage::test_get_dataset_files

This test fails locally. I took a look and here's the issue:

  File "/Users/tyu/anaconda3/envs/schematic/lib/python3.10/site-packages/flask/app.py", line 1517, in full_dispatch_request
    rv = self.dispatch_request()
  File "/Users/tyu/anaconda3/envs/schematic/lib/python3.10/site-packages/flask/app.py", line 1503, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**req.view_args)
  File "/Users/tyu/anaconda3/envs/schematic/lib/python3.10/site-packages/connexion/decorators/decorator.py", line 68, in wrapper
    response = function(request)
  File "/Users/tyu/anaconda3/envs/schematic/lib/python3.10/site-packages/connexion/security/security_handler_factory.py", line 388, in wrapper
    return function(request)
  File "/Users/tyu/anaconda3/envs/schematic/lib/python3.10/site-packages/connexion/decorators/uri_parsing.py", line 149, in wrapper
    response = function(request)
  File "/Users/tyu/anaconda3/envs/schematic/lib/python3.10/site-packages/connexion/decorators/validation.py", line 399, in wrapper
    return function(request)
  File "/Users/tyu/anaconda3/envs/schematic/lib/python3.10/site-packages/connexion/decorators/parameter.py", line 120, in wrapper
    return function(**kwargs)
  File "/Users/tyu/sage/schematic/schematic_api/api/routes.py", line 533, in get_files_storage_dataset
    file_lst = store.getFilesInStorageDataset(
  File "/Users/tyu/anaconda3/envs/schematic/lib/python3.10/contextlib.py", line 79, in inner
    return func(*args, **kwds)
  File "/Users/tyu/sage/schematic/schematic/store/synapse.py", line 710, in getFilesInStorageDataset
    dataset_path = f"'{self.storageFileviewTable.loc[self.storageFileviewTable['id']==datasetId,'path'][0]}/%'"
  File "/Users/tyu/anaconda3/envs/schematic/lib/python3.10/site-packages/pandas/core/series.py", line 1118, in __getitem__
    return self._values[key]
IndexError: index 0 is out of bounds for axis 0 with size 0

schematic/store/synapse.py

Co-authored-by: Thomas Yu <[email protected]>

GiaJordan · 2024-11-22T17:40:55Z

Okay with that last comment I think that's everything provided the tests pass.
@BryanFauble and @thomasyu888 I've re-requested your review

schematic/store/synapse.py

sonarcloud · 2024-11-23T01:26:08Z

Quality Gate passed

Issues
6 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

thomasyu888 · 2024-11-23T02:21:02Z

I took a deeper look at the test_store.py, please take a look: #1554. re: failing tests.

GiaJordan added 8 commits October 29, 2024 10:56

add test for clause method

8a80ce3

add method to process dataset id into query clause

a2de0c4

use new method for validation

45193a1

update clause method

80b6bda

update file based manifest gen test

d810576

consolidate filebased manifest gen tests

b1e60af

update test layout

33796b4

use fileview for file paths

62d8b28

GiaJordan requested review from andrewelamb, linglp and BryanFauble as code owners October 30, 2024 20:35

GiaJordan marked this pull request as draft October 30, 2024 20:35

GiaJordan changed the title ~~Fds 2293 file paths for manifest gen~~ Use paths from file view for manifest generation Oct 30, 2024

GiaJordan and others added 17 commits October 31, 2024 09:19

add functionality to just return filename

8b012d3

make syn id regex a util function

8e670ad

add non-api integration test for detFilesInStorageDataset

c90ebf0

fix typo and mismatched ids

f9b9bcc

add case for nested data structure

be4f6b6

get nested files as well

b39afe2

add return type annotation

566e741

add docstring

86fb15d

add str prefix

2fa1ad6

revert param name

cd6713b

change datasetid clause method

9438811

get files in doubly+ nested files

e076770

add comments

19188ea

add test cases for filtering results

c46a100

add test case for filtered results

ccead76

Update README.md

26e5539

Update README.md

2f835bd

GiaJordan added 2 commits November 19, 2024 13:50

change test name case

bc9f479

update descriptions

79f5bda

thomasyu888 reviewed Nov 20, 2024

View reviewed changes

GiaJordan added 4 commits November 20, 2024 10:10

undo development change

5d0599b

add comment

c1cd79a

remove dev work

feb82e8

remove temp test marks

c1587c6

GiaJordan added 4 commits November 20, 2024 12:08

update test for new expected order

2769d6a

change method for gathering files in a dataset

dd9d6a9

update mock test

800a4cd

update other mocked test

95cfeed

thomasyu888 requested changes Nov 22, 2024

View reviewed changes

thomasyu888 reviewed Nov 22, 2024

View reviewed changes

schematic/store/synapse.py Outdated Show resolved Hide resolved

GiaJordan added 2 commits November 22, 2024 09:46

change method for building dataset path

9701078

wrap path in quotes

7cd0ce0

thomasyu888 reviewed Nov 22, 2024

View reviewed changes

schematic/store/synapse.py Outdated Show resolved Hide resolved

update quotes for dataset path

65dcc35

Co-authored-by: Thomas Yu <[email protected]>

GiaJordan requested review from thomasyu888 and BryanFauble November 22, 2024 17:40

BryanFauble reviewed Nov 22, 2024

View reviewed changes

schematic/store/synapse.py Outdated Show resolved Hide resolved

reformat and add exception

f9d037d

thomasyu888 reviewed Nov 22, 2024

View reviewed changes

schematic/store/synapse.py Show resolved Hide resolved

add unit test

e07f973

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SCHEMATIC-183] Use paths from file view for manifest generation #1529

[SCHEMATIC-183] Use paths from file view for manifest generation #1529

GiaJordan commented Oct 30, 2024 •

edited

Loading

thomasyu888 Nov 20, 2024

GiaJordan commented Nov 20, 2024

GiaJordan commented Nov 21, 2024 •

edited

Loading

BryanFauble commented Nov 21, 2024

GiaJordan commented Nov 21, 2024

thomasyu888 left a comment

GiaJordan commented Nov 22, 2024

sonarcloud bot commented Nov 23, 2024

thomasyu888 commented Nov 23, 2024 •

edited

Loading

[SCHEMATIC-183] Use paths from file view for manifest generation #1529

Are you sure you want to change the base?

[SCHEMATIC-183] Use paths from file view for manifest generation #1529

Conversation

GiaJordan commented Oct 30, 2024 • edited Loading

thomasyu888 Nov 20, 2024

Choose a reason for hiding this comment

GiaJordan commented Nov 20, 2024

GiaJordan commented Nov 21, 2024 • edited Loading

BryanFauble commented Nov 21, 2024

GiaJordan commented Nov 21, 2024

thomasyu888 left a comment

Choose a reason for hiding this comment

GiaJordan commented Nov 22, 2024

sonarcloud bot commented Nov 23, 2024

Quality Gate passed

thomasyu888 commented Nov 23, 2024 • edited Loading

GiaJordan commented Oct 30, 2024 •

edited

Loading

GiaJordan commented Nov 21, 2024 •

edited

Loading

thomasyu888 commented Nov 23, 2024 •

edited

Loading