Skip to content

Commit a64dc79

Browse files
mohammad-alisafaeejsam
mohammad-alisafaee
authored and
jsam
committed
fix: only check local repo for lfs filter (#575)
1 parent f60783e commit a64dc79

File tree

2 files changed

+98
-74
lines changed

2 files changed

+98
-74
lines changed

‎renku/api/storage.py

Lines changed: 86 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -65,99 +65,116 @@ def init_external_storage(self, force=False):
6565
@property
6666
def external_storage_installed(self):
6767
"""Check that Large File Storage is installed."""
68-
return HAS_LFS and self.repo.config_reader(
69-
).has_section('filter "lfs"')
68+
return HAS_LFS
7069

7170
def track_paths_in_storage(self, *paths):
7271
"""Track paths in the external storage."""
73-
if self.use_external_storage and self.external_storage_installed:
74-
track_paths = []
75-
attrs = self.find_attr(*paths)
76-
77-
for path in paths:
78-
# Do not add files with filter=lfs in .gitattributes
79-
if attrs.get(path, {}).get('filter') == 'lfs':
80-
continue
81-
82-
path = Path(path)
83-
if path.is_dir():
84-
track_paths.append(str(path / '**'))
85-
elif path.suffix != '.ipynb':
86-
# TODO create configurable filter and follow .gitattributes
87-
track_paths.append(str(path))
88-
89-
call(
90-
self._CMD_STORAGE_TRACK + track_paths,
91-
stdout=PIPE,
92-
stderr=STDOUT,
93-
cwd=str(self.path),
94-
)
95-
elif self.use_external_storage:
72+
if not self._use_lfs():
73+
return
74+
75+
if not self.external_storage_installed:
9676
raise errors.ExternalStorageNotInstalled(self.repo)
9777

78+
track_paths = []
79+
attrs = self.find_attr(*paths)
80+
81+
for path in paths:
82+
# Do not add files with filter=lfs in .gitattributes
83+
if attrs.get(path, {}).get('filter') == 'lfs':
84+
continue
85+
86+
path = Path(path)
87+
if path.is_dir():
88+
track_paths.append(str(path / '**'))
89+
elif path.suffix != '.ipynb':
90+
# TODO create configurable filter and follow .gitattributes
91+
track_paths.append(str(path))
92+
93+
call(
94+
self._CMD_STORAGE_TRACK + track_paths,
95+
stdout=PIPE,
96+
stderr=STDOUT,
97+
cwd=str(self.path),
98+
)
99+
98100
def untrack_paths_from_storage(self, *paths):
99101
"""Untrack paths from the external storage."""
100-
if self.use_external_storage and self.external_storage_installed:
101-
call(
102-
self._CMD_STORAGE_UNTRACK + list(paths),
103-
stdout=PIPE,
104-
stderr=STDOUT,
105-
cwd=str(self.path),
106-
)
107-
elif self.use_external_storage:
102+
if not self._use_lfs():
103+
return
104+
105+
if not self.external_storage_installed:
108106
raise errors.ExternalStorageNotInstalled(self.repo)
109107

108+
call(
109+
self._CMD_STORAGE_UNTRACK + list(paths),
110+
stdout=PIPE,
111+
stderr=STDOUT,
112+
cwd=str(self.path),
113+
)
114+
110115
def pull_paths_from_storage(self, *paths):
111116
"""Pull paths from LFS."""
112117
import math
113-
if self.use_external_storage and self.external_storage_installed:
114-
client_dict = defaultdict(list)
115118

116-
for path in _expand_directories(paths):
117-
client, commit, path = self.resolve_in_submodules(
118-
self.repo.commit(), path
119-
)
120-
client_dict[client.path].append(str(path))
121-
122-
for client_path, paths in client_dict.items():
123-
for ibatch in range(
124-
math.ceil(len(paths) / ARGUMENT_BATCH_SIZE)
125-
):
126-
run(
127-
self._CMD_STORAGE_PULL + [
128-
shlex.quote(
129-
','.join(
130-
paths[ibatch * ARGUMENT_BATCH_SIZE:
131-
(ibatch + 1) * ARGUMENT_BATCH_SIZE]
132-
)
133-
)
134-
],
135-
cwd=str(client_path.absolute()),
136-
stdout=PIPE,
137-
stderr=STDOUT,
138-
)
139-
elif self.use_external_storage:
119+
if not self._use_lfs():
120+
return
121+
122+
if not self.external_storage_installed:
140123
raise errors.ExternalStorageNotInstalled(self.repo)
141124

125+
client_dict = defaultdict(list)
126+
127+
for path in _expand_directories(paths):
128+
client, commit, path = self.resolve_in_submodules(
129+
self.repo.commit(), path
130+
)
131+
client_dict[client.path].append(str(path))
132+
133+
for client_path, paths in client_dict.items():
134+
for ibatch in range(math.ceil(len(paths) / ARGUMENT_BATCH_SIZE)):
135+
run(
136+
self._CMD_STORAGE_PULL + [
137+
shlex.quote(
138+
','.join(
139+
paths[ibatch * ARGUMENT_BATCH_SIZE:
140+
(ibatch + 1) * ARGUMENT_BATCH_SIZE]
141+
)
142+
)
143+
],
144+
cwd=str(client_path.absolute()),
145+
stdout=PIPE,
146+
stderr=STDOUT,
147+
)
148+
142149
def checkout_paths_from_storage(self, *paths):
143150
"""Checkout a paths from LFS."""
144-
if self.use_external_storage and self.external_storage_installed:
145-
run(
146-
self._CMD_STORAGE_CHECKOUT + list(paths),
147-
cwd=str(self.path.absolute()),
148-
stdout=PIPE,
149-
stderr=STDOUT,
150-
check=True,
151-
)
152-
elif self.use_external_storage:
151+
if not self._use_lfs():
152+
return
153+
154+
if not self.external_storage_installed:
153155
raise errors.ExternalStorageNotInstalled(self.repo)
154156

157+
run(
158+
self._CMD_STORAGE_CHECKOUT + list(paths),
159+
cwd=str(self.path.absolute()),
160+
stdout=PIPE,
161+
stderr=STDOUT,
162+
check=True,
163+
)
164+
155165
def init_repository(self, name=None, force=False):
156166
"""Initialize a local Renku repository."""
157167
result = super().init_repository(name=name, force=force)
158168

159169
# initialize LFS if it is requested and installed
160-
if self.use_external_storage and HAS_LFS:
170+
if self.use_external_storage and self.external_storage_installed:
161171
self.init_external_storage(force=force)
162172

163173
return result
174+
175+
def _use_lfs(self):
176+
renku_initialized_to_use_lfs = self.repo.config_reader(
177+
config_level='repository'
178+
).has_section('filter "lfs"')
179+
180+
return renku_initialized_to_use_lfs and self.use_external_storage

‎tests/test_cli.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -397,16 +397,23 @@ def test_configuration_of_external_storage(isolated_runner, monkeypatch):
397397
m.setattr(StorageApiMixin, 'external_storage_installed', False)
398398

399399
result = runner.invoke(cli.cli, ['run', 'touch', 'output'])
400-
assert 1 == result.exit_code
401-
subprocess.call(['git', 'clean', '-df'])
400+
assert 0 == result.exit_code
402401

403-
result = runner.invoke(cli.cli, ['-S', 'run', 'touch', 'output'])
402+
result = runner.invoke(cli.cli, ['-S', 'run', 'touch', 'output2'])
404403
assert 0 == result.exit_code
405404

406405
result = runner.invoke(cli.cli, ['init', '--force'])
407406
assert 0 == result.exit_code
408407

409-
result = runner.invoke(cli.cli, ['run', 'touch', 'output2'])
408+
with monkeypatch.context() as m:
409+
from renku.api.storage import StorageApiMixin
410+
m.setattr(StorageApiMixin, 'external_storage_installed', False)
411+
412+
result = runner.invoke(cli.cli, ['run', 'touch', 'output3'])
413+
assert 1 == result.exit_code
414+
subprocess.call(['git', 'clean', '-df'])
415+
416+
result = runner.invoke(cli.cli, ['run', 'touch', 'output4'])
410417
assert 0 == result.exit_code
411418

412419

@@ -489,7 +496,7 @@ def test_status_with_submodules(isolated_runner, monkeypatch):
489496
cli.cli, ['dataset', 'add', 'f', '../woop'],
490497
catch_exceptions=False
491498
)
492-
assert 1 == result.exit_code
499+
assert 0 == result.exit_code
493500
subprocess.call(['git', 'clean', '-dff'])
494501

495502
result = runner.invoke(

0 commit comments

Comments
 (0)