Skip to content

Commit 52859b0

Browse files
authored
Fix workflow problems for Persistеnce test executions and ASV AWS S3 tests execution (#2585)
#### Reference Issues/PRs <!--Example: Fixes #1234. See also #3456.--> #### What does this implement or fix? ASV tests problems with AWS S3: the code wrongly assumed aws bucket from azure env vars. This works locally as I did had Azure vars. But the ASW workflow does not supply those vars. This is fixed and run is initiated from Actions: https://github.com/man-group/ArcticDB/actions/runs/16987111363 Persistance tests are executed from a separate workflow. This workflow runc cleanup seed and verify tasks. The workflow does not install any arcticdb test libraries. Further it works with old arctic also, therefore it is much more closer to installation tests and must not include any new stuff from master but stuff that has been there in 3.0.0 only. This again was invisible requirement during development of Azure. Thus all worked fine Run from Actions with AWS_S3: https://github.com/man-group/ArcticDB/actions/runs/16988249102 Both could only be revealed if executed against latest changed sources from Azure branc. As those workflows are not part pf the PR they were forgotten .... #### Any other comments? #### Checklist <details> <summary> Checklist for code changes... </summary> - [ ] Have you updated the relevant docstrings, documentation and copyright notice? - [ ] Is this contribution tested against [all ArcticDB's features](../docs/mkdocs/docs/technical/contributing.md)? - [ ] Do all exceptions introduced raise appropriate [error messages](https://docs.arcticdb.io/error_messages/)? - [ ] Are API changes highlighted in the PR description? - [ ] Is the PR labelled as enhancement or bug so it appears in autogenerated release notes? </details> <!-- Thanks for contributing a Pull Request to ArcticDB! Please ensure you have taken a look at: - ArcticDB's Code of Conduct: https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md - ArcticDB's Contribution Licensing: https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing --> --------- Co-authored-by: Georgi Rusev <Georgi Rusev>
1 parent dd746f0 commit 52859b0

File tree

4 files changed

+63
-10
lines changed

4 files changed

+63
-10
lines changed

.github/workflows/build.yml

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ on:
2121
- 'GCPXML'
2222
- 'AZURE'
2323
default: 'no'
24-
2524
pypi_publish:
2625
type: boolean
2726
publish_env:
@@ -230,6 +229,7 @@ jobs:
230229
secrets: inherit
231230
with:
232231
job_type: cleanup
232+
persistent_storage: ${{ needs.storage_type.outputs.storage }}
233233

234234
persistent_storage_seed_linux:
235235
needs: [common_config, pre_seed_cleanup, storage_type]
@@ -254,6 +254,8 @@ jobs:
254254
arcticdb_version: ${{matrix.arcticdb_version}}
255255
matrix: ${{toJson(matrix.matrix_override)}}
256256
python_deps_ids: ${{toJson(matrix.python_deps_ids)}}
257+
persistent_storage: ${{ needs.storage_type.outputs.storage }}
258+
257259

258260
persistent_storage_seed_windows:
259261
needs: [common_config, pre_seed_cleanup, storage_type]
@@ -278,6 +280,7 @@ jobs:
278280
arcticdb_version: ${{matrix.arcticdb_version}}
279281
matrix: ${{toJson(matrix.matrix_override)}}
280282
python_deps_ids: ${{toJson(matrix.python_deps_ids)}}
283+
persistent_storage: ${{ needs.storage_type.outputs.storage }}
281284

282285
cpp-test-linux:
283286
needs: [cibw_docker_image, common_config]
@@ -387,7 +390,7 @@ jobs:
387390
DEBUG_LOGGING_ENABLED: ${{ needs.storage_type.outputs.DEBUG_LOGGING_ENABLED }}
388391

389392
persistent_storage_verify_linux:
390-
needs: [common_config, build-python-wheels-linux, build-python-wheels-windows]
393+
needs: [common_config, build-python-wheels-linux, build-python-wheels-windows, storage_type]
391394
# This step is valid only for AWS_S3.
392395
if: ${{ needs.storage_type.outputs.storage == 'AWS_S3'}}
393396
strategy:
@@ -409,8 +412,10 @@ jobs:
409412
arcticdb_version: ${{matrix.arcticdb_version}}
410413
matrix: ${{toJson(matrix.matrix_override)}}
411414
python_deps_ids: ${{toJson(matrix.python_deps_ids)}}
415+
persistent_storage: ${{ needs.storage_type.outputs.storage }}
412416

413417
persistent_storage_verify_windows:
418+
needs: [common_config, storage_type]
414419
# This step is valid only for AWS_S3.
415420
if: ${{ needs.storage_type.outputs.storage == 'AWS_S3'}}
416421
strategy:
@@ -432,6 +437,7 @@ jobs:
432437
arcticdb_version: ${{matrix.arcticdb_version}}
433438
matrix: ${{toJson(matrix.matrix_override)}}
434439
python_deps_ids: ${{toJson(matrix.python_deps_ids)}}
440+
persistent_storage: ${{ needs.storage_type.outputs.storage }}
435441

436442
post_verify_cleanup:
437443
needs: [persistent_storage_verify_windows, persistent_storage_verify_linux, storage_type]
@@ -446,6 +452,7 @@ jobs:
446452
secrets: inherit
447453
with:
448454
job_type: cleanup
455+
persistent_storage: ${{ needs.storage_type.outputs.storage }}
449456

450457
build-python-wheels-macos:
451458
needs: [common_config, storage_type]

.github/workflows/persistent_storage.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ on:
1212
endpoint: {default: 's3.eu-west-1.amazonaws.com', type: string, description: The address of the S3 endpoint}
1313
region: {default: 'eu-west-1', type: string, description: The S3 region of the bucket}
1414
clear: {default: 0, type: number, description: Controls wheather we will clear the libraries in the versions stores that we test against}
15+
persistent_storage: {required: true, default: "no", type: string, description: Specifies whether the python tests should tests against real storages e.g. AWS S3 }
16+
1517
jobs:
1618
setup:
1719
if: inputs.job_type != 'cleanup'
@@ -75,6 +77,7 @@ jobs:
7577
aws_access_key: "${{ secrets.AWS_S3_ACCESS_KEY }}"
7678
aws_secret_key: "${{ secrets.AWS_S3_SECRET_KEY }}"
7779
strategy_branch: "${{matrix.os}}_${{env.python_impl_name}}"
80+
persistent_storage: ${{ inputs.persistent_storage }}
7881

7982
- name: Seed the storage
8083
if: inputs.job_type == 'seed'
@@ -109,6 +112,7 @@ jobs:
109112
with:
110113
aws_access_key: "${{ secrets.AWS_S3_ACCESS_KEY }}"
111114
aws_secret_key: "${{ secrets.AWS_S3_SECRET_KEY }}"
115+
persistent_storage: ${{ inputs.persistent_storage }}
112116

113117
- name: Clean the storage
114118
run: |

python/arcticdb/util/environment_setup.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,9 @@ def __new__(cls, *args, **kwargs):
9191

9292
# Azure variable setup
9393
cls._azure_factory = real_azure_from_environment_variables(shared_path=True)
94-
cls._aws_default_factory.default_prefix = None
95-
cls._aws_default_factory.default_bucket = AZURE_DEFAULT_CONTAINER
96-
cls._aws_default_factory.clean_bucket_on_fixture_exit = False
94+
cls._azure_factory.default_prefix = None
95+
cls._azure_factory.default_container = AZURE_DEFAULT_CONTAINER
96+
cls._azure_factory.clean_bucket_on_fixture_exit = False
9797

9898

9999
@classmethod

python/tests/util/storage_test.py

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from enum import Enum
2+
import sys
23
import pandas as pd
34
from pandas.testing import assert_frame_equal
45
import os
@@ -10,7 +11,6 @@
1011

1112
from arcticdb import Arctic
1213
from arcticc.pb2.s3_storage_pb2 import Config as S3Config
13-
from arcticdb.storage_fixtures.azure import real_azure_from_environment_variables
1414
try:
1515
# from pytest this way will work
1616
from tests.util.mark import PERSISTENT_STORAGE_TESTS_ENABLED
@@ -96,6 +96,19 @@ def real_gcp_credentials(shared_path: bool = True):
9696
return endpoint, bucket, region, access_key, secret_key, path_prefix, clear
9797

9898

99+
def real_azure_credentials(shared_path: bool = True):
100+
if shared_path:
101+
path_prefix = os.getenv("ARCTICDB_PERSISTENT_STORAGE_SHARED_PATH_PREFIX")
102+
else:
103+
path_prefix = os.getenv("ARCTICDB_PERSISTENT_STORAGE_UNIQUE_PATH_PREFIX", "")
104+
constr=os.getenv("ARCTICDB_REAL_AZURE_CONNECTION_STRING"),
105+
container=os.getenv("ARCTICDB_REAL_AZURE_CONTAINER"),
106+
107+
clear = str(os.getenv("ARCTICDB_REALL_AZURE_CLEAR")).lower() in ("true", "1")
108+
109+
return container, constr, path_prefix, clear
110+
111+
99112
def get_real_s3_uri(shared_path: bool = True):
100113
# TODO: Remove this when the latest version that we support
101114
# contains the s3 fixture code as defined here:
@@ -133,10 +146,38 @@ def get_real_gcp_uri(shared_path: bool = True):
133146
)
134147
return aws_uri
135148

149+
def find_ca_certs():
150+
# Common CA certificates locations
151+
default_paths = ssl.get_default_verify_paths()
152+
possible_paths = [
153+
default_paths.cafile,
154+
default_paths.openssl_cafile_env,
155+
default_paths.openssl_cafile,
156+
'/etc/ssl/certs/ca-certificates.crt',
157+
'/usr/lib/ssl/certs/ca-certificates.crt',
158+
'/etc/pki/tls/certs/ca-bundle.crt',
159+
'/etc/ssl/cert.pem'
160+
]
161+
for path in possible_paths:
162+
if path and os.path.isfile(path):
163+
return path
164+
return None
165+
166+
### IMPORTANT: When adding new STORAGE we must implement
167+
### the whole connection logic here even if this does mean effectively duplicating the code
168+
###
169+
### REASON: We run this file from command line on arcticdb version 3.0.
170+
### there is no way how arcticdb 3.0 could have had the functions that we are going to implement
171+
### and support from now on
136172
def get_real_azure_uri(shared_path: bool = True):
137-
return real_azure_from_environment_variables(
138-
shared_path=shared_path,
139-
).get_arctic_uri()
173+
container, constr, path_prefix = real_azure_credentials(shared_path)
174+
ca_certs_file = find_ca_certs()
175+
uri = f"azure://Container={container};Path_prefix={path_prefix}"
176+
assert ca_certs_file, f"CA file: {ca_certs_file} not found!"
177+
if sys.platform.lower().startswith("linux"):
178+
uri += f";CA_cert_path={ca_certs_file}"
179+
url += f";{constr}"
180+
return url
140181

141182

142183
class PersistentTestType(Enum):
@@ -159,7 +200,8 @@ def persistent_test_type() -> PersistentTestType:
159200
return PersistentTestType.AZURE
160201
return PersistentTestType.AWS_S3
161202
else:
162-
raise Exception("Persistence storage tests are not enabled or not configured properly")
203+
raise Exception("Persistence storage tests are not enabled or not configured properly."
204+
+ "ARCTICDB_PERSISTENT_STORAGE_TESTS_ENABLED environment variable is not set")
163205

164206

165207
def get_real_uri(shared_path: bool = True):

0 commit comments

Comments
 (0)