Skip to content

Commit 44e95b3

Browse files
committed
Return a presigned URL
1 parent 38763cd commit 44e95b3

2 files changed

Lines changed: 74 additions & 12 deletions

File tree

src/nomad_ml_workflows/actions/export_remote_entries/activities.py

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,33 @@ def _unique_upload_filename(filename: str, upload_files: StagingUploadFiles) ->
105105
count += 1
106106

107107

108-
def _parse_s3_uri(remote_uri: str) -> tuple[str, str]:
109-
"""Parse an S3 URI into its bucket and object key/prefix."""
108+
DEFAULT_PRESIGNED_URL_EXPIRATION_SECONDS = 2 * 24 * 3600 # 2 days (172800s)
109+
110+
111+
def _parse_s3_uri(
112+
remote_uri: str, default_bucket: str | None = None
113+
) -> tuple[str, str]:
114+
"""Parse an S3 URI or presigned URL into its bucket and object key/prefix."""
110115
parsed_uri = urlparse(remote_uri)
111-
key = parsed_uri.path.lstrip('/')
112-
if parsed_uri.scheme != 's3' or not parsed_uri.netloc or not key:
113-
raise ValueError(f'Invalid S3 URI: {remote_uri}')
114-
return parsed_uri.netloc, key
116+
if parsed_uri.scheme == 's3':
117+
key = parsed_uri.path.lstrip('/')
118+
if not parsed_uri.netloc or not key:
119+
raise ValueError(f'Invalid S3 URI: {remote_uri}')
120+
return parsed_uri.netloc, key
121+
elif parsed_uri.scheme in ('http', 'https'):
122+
path = parsed_uri.path.lstrip('/')
123+
if default_bucket and path.startswith(f'{default_bucket}/'):
124+
key = path[len(default_bucket) + 1 :]
125+
return default_bucket, key
126+
elif default_bucket:
127+
return default_bucket, path
128+
parts = path.split('/', 1)
129+
if len(parts) > 1:
130+
return parts[0], parts[1]
131+
elif parts and parts[0]:
132+
return parsed_uri.netloc, parts[0]
133+
raise ValueError(f'Could not determine S3 bucket/key from URL: {remote_uri}')
134+
raise ValueError(f'Invalid S3 URI: {remote_uri}')
115135

116136

117137
def _upload_dataset_to_s3(
@@ -120,7 +140,7 @@ def _upload_dataset_to_s3(
120140
exportable_filepaths: list[Path],
121141
artifacts_subdirectory: Path,
122142
) -> str:
123-
"""Upload exported dataset files to S3-compatible remote storage."""
143+
"""Upload exported dataset files to S3-compatible remote storage and return a presigned download URL."""
124144
import boto3
125145

126146
client_kwargs = _build_boto3_client_kwargs(storage_settings)
@@ -136,12 +156,32 @@ def _upload_dataset_to_s3(
136156

137157
object_key = _build_s3_key(prefix, f'{data.exportable_dir_name}.zip')
138158
s3_client.upload_file(zippath.as_posix(), bucket, object_key)
139-
return f's3://{bucket}/{object_key}'
159+
try:
160+
return s3_client.generate_presigned_url(
161+
'get_object',
162+
Params={'Bucket': bucket, 'Key': object_key},
163+
ExpiresIn=DEFAULT_PRESIGNED_URL_EXPIRATION_SECONDS,
164+
)
165+
except Exception:
166+
return f's3://{bucket}/{object_key}'
140167

141168
base_key_prefix = _build_s3_key(prefix, data.exportable_dir_name)
169+
primary_object_key = None
142170
for filepath in exportable_filepaths:
143171
object_key = f'{base_key_prefix}/{filepath.name}'
144172
s3_client.upload_file(filepath.as_posix(), bucket, object_key)
173+
if filepath.stem == DATA_FILE_NAME or primary_object_key is None:
174+
primary_object_key = object_key
175+
176+
if primary_object_key:
177+
try:
178+
return s3_client.generate_presigned_url(
179+
'get_object',
180+
Params={'Bucket': bucket, 'Key': primary_object_key},
181+
ExpiresIn=DEFAULT_PRESIGNED_URL_EXPIRATION_SECONDS,
182+
)
183+
except Exception:
184+
pass
145185

146186
return f's3://{bucket}/{base_key_prefix}/'
147187

@@ -153,7 +193,7 @@ def _copy_dataset_from_s3_to_upload(
153193
"""Download an S3 dataset and add it to a NOMAD staging upload."""
154194
import boto3
155195

156-
bucket, key = _parse_s3_uri(data.remote_uri)
196+
bucket, key = _parse_s3_uri(data.remote_uri, default_bucket=storage_settings.bucket)
157197
client_kwargs = _build_boto3_client_kwargs(storage_settings)
158198
s3_client = boto3.client('s3', **client_kwargs)
159199
upload_files = _get_staging_upload_files(data.user_id, data.upload_id)

tests/actions/export_remote_entries/test_activities.py

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ async def test_upload_dataset_to_remote_storage_s3_zipped(
4848
mock_artifacts_dir.return_value = artifacts_dir.as_posix()
4949

5050
mock_s3 = MagicMock()
51+
mock_s3.generate_presigned_url.return_value = (
52+
'https://my-bucket.s3.amazonaws.com/my-folder/export_entries_2026.zip?signed'
53+
)
5154
mock_boto_client.return_value = mock_s3
5255

5356
storage_settings = S3StorageSettings(
@@ -84,7 +87,15 @@ async def test_upload_dataset_to_remote_storage_s3_zipped(
8487
call_args = mock_s3.upload_file.call_args[0]
8588
assert call_args[1] == 'my-bucket'
8689
assert call_args[2] == 'my-folder/export_entries_2026.zip'
87-
assert remote_uri == 's3://my-bucket/my-folder/export_entries_2026.zip'
90+
mock_s3.generate_presigned_url.assert_called_once_with(
91+
'get_object',
92+
Params={'Bucket': 'my-bucket', 'Key': 'my-folder/export_entries_2026.zip'},
93+
ExpiresIn=172800,
94+
)
95+
assert (
96+
remote_uri
97+
== 'https://my-bucket.s3.amazonaws.com/my-folder/export_entries_2026.zip?signed'
98+
)
8899

89100

90101
@pytest.mark.asyncio
@@ -103,6 +114,9 @@ async def test_upload_dataset_to_remote_storage_s3_unzipped(
103114
mock_artifacts_dir.return_value = artifacts_dir.as_posix()
104115

105116
mock_s3 = MagicMock()
117+
mock_s3.generate_presigned_url.return_value = (
118+
'https://my-bucket.s3.amazonaws.com/export_entries_2026/data.csv?signed'
119+
)
106120
mock_boto_client.return_value = mock_s3
107121

108122
storage_settings = S3StorageSettings(
@@ -125,7 +139,15 @@ async def test_upload_dataset_to_remote_storage_s3_unzipped(
125139
assert 'export_entries_2026/metadata.json' in uploaded_keys
126140
assert 'export_entries_2026/selected_entries.json' in uploaded_keys
127141
assert 'export_entries_2026/data.csv' in uploaded_keys
128-
assert remote_uri == 's3://my-bucket/export_entries_2026/'
142+
mock_s3.generate_presigned_url.assert_called_once_with(
143+
'get_object',
144+
Params={'Bucket': 'my-bucket', 'Key': 'export_entries_2026/data.csv'},
145+
ExpiresIn=172800,
146+
)
147+
assert (
148+
remote_uri
149+
== 'https://my-bucket.s3.amazonaws.com/export_entries_2026/data.csv?signed'
150+
)
129151

130152

131153
@pytest.mark.asyncio
@@ -144,7 +166,7 @@ async def test_copy_remote_dataset_to_upload_s3_zipped(
144166
data = CopyRemoteDatasetToUploadInput(
145167
user_id='user-123',
146168
upload_id='upload-123',
147-
remote_uri='s3://my-bucket/exports/dataset.zip',
169+
remote_uri='https://s3.example.com/my-bucket/exports/dataset.zip?X-Amz-Signature=123',
148170
storage_settings=S3StorageSettings(
149171
bucket='my-bucket',
150172
endpoint_url='https://s3.example.com',

0 commit comments

Comments
 (0)