Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Toniof 516 unify datasetio #527

Merged
merged 14 commits into from
Sep 21, 2021
Merged

Toniof 516 unify datasetio #527

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
7007df1
work on integrating store framework into dataset io
TonioF Sep 9, 2021
e66209e
intermediate working state
TonioF Sep 10, 2021
eaa047e
context makes more use of datastore framework
TonioF Sep 10, 2021
b5c8197
removed removed methods
TonioF Sep 13, 2021
dcd83f9
use path as required parameter
TonioF Sep 13, 2021
8f66919
path fixes
TonioF Sep 13, 2021
8f7ba10
context uses data store framework
TonioF Sep 13, 2021
03826ed
Merge remote-tracking branch 'origin/master' into toniof-516-unify-da…
TonioF Sep 14, 2021
1ae009f
test fixes
TonioF Sep 14, 2021
c50137c
considered pr review
TonioF Sep 14, 2021
e2f08cf
updated config
TonioF Sep 14, 2021
b272243
Merge branch 'master' into toniof-516-unify-datasetio
TonioF Sep 14, 2021
b1f7ccf
updated config
TonioF Sep 14, 2021
9f49034
considered pr review
TonioF Sep 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/serve/demo/config-with-stores.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ DataStores:
# client_kwargs:
# endpoint_url: https://s3.eu-central-1.amazonaws.com
Datasets:
- Identifier: "*.zarr"
- Path: "*.zarr"
Style: "default"
# ChunkCacheSize: 1G

Expand Down
30 changes: 29 additions & 1 deletion test/core/store/test_storepool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,34 @@ def test_multi_stores_with_params(self):
self.assertIsInstance(pool, DataStorePool)
self.assertEqual(["local-1", "local-2", "ram-1", "ram-2"], pool.store_instance_ids)
for instance_id in pool.store_instance_ids:
self.assertTrue(pool.has_store_config(instance_id))
self.assertTrue(pool.has_store_instance(instance_id))
self.assertIsInstance(pool.get_store_config(instance_id), DataStoreConfig)
self.assertIsInstance(pool.get_store(instance_id), DataStore)

def test_get_store_instance_id(self):
store_params_1 = {
"root": "./bibo"
}
ds_config_1 = DataStoreConfig(store_id='file',
store_params=store_params_1)
ds_configs = {'dir-1': ds_config_1}
pool = DataStorePool(ds_configs)

store_params_2 = {
"root": "./babo"
}
ds_config_2 = DataStoreConfig(store_id='file',
store_params=store_params_2)
ds_config_3 = DataStoreConfig(store_id='file',
store_params=store_params_1,
title='A third configuration')

self.assertEqual('dir-1', pool.get_store_instance_id(ds_config_1))
self.assertEqual('dir-1', pool.get_store_instance_id(ds_config_1,
strict_check=True))

self.assertIsNone(pool.get_store_instance_id(ds_config_2))

self.assertEqual('dir-1', pool.get_store_instance_id(ds_config_3))
self.assertIsNone(pool.get_store_instance_id(ds_config_3,
strict_check=True))
2 changes: 1 addition & 1 deletion test/webapi/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def test_from_dict(self):
},
"Datasets": [
{
"Identifier": "*.zarr",
"Path": "*.zarr",
"Style": "default"
}
]
Expand Down
226 changes: 226 additions & 0 deletions test/webapi/test_context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,229 @@ def test_interpolates_vars(self):
normalize_prefix('/${name}'))
self.assertEqual(f'/xcube/v{version}',
normalize_prefix('/${name}/v${version}'))


class MaybeAssignStoreInstanceIdsTest(unittest.TestCase):

def test_find_common_store(self):
ctx = new_test_service_context()
dataset_configs = [
{
'Identifier': 'z_0',
'FileSystem': 'local',
'Path': '/one/path/abc.zarr'
},
{
'Identifier': 'z_1',
'FileSystem': 'local',
'Path': '/one/path/def.zarr'
},
{
'Identifier': 'z_4',
'FileSystem': 'obs',
'Path': '/one/path/mno.zarr'
},
{
'Identifier': 'z_2',
'FileSystem': 'local',
'Path': '/another/path/ghi.zarr'
},
{
'Identifier': 'z_3',
'FileSystem': 'local',
'Path': '/one/more/path/jkl.zarr'
},
{
'Identifier': 'z_5',
'FileSystem': 'obs',
'Path': '/one/path/pqr.zarr'
},
{
'Identifier': 'z_6',
'FileSystem': 'local',
'Path': '/one/path/stu.zarr'
},
{
'Identifier': 'z_7',
'FileSystem': 'local',
'Path': '/one/more/path/vwx.zarr'
},
]
ctx.config['Datasets'] = dataset_configs
adjusted_dataset_configs = ctx.get_dataset_configs()

expected_dataset_configs = [
{
'Identifier': 'z_0',
'FileSystem': 'local',
'Path': 'path/abc.zarr',
'StoreInstanceId': 'local_2'
},
{
'Identifier': 'z_1',
'FileSystem': 'local',
'Path': 'path/def.zarr',
'StoreInstanceId': 'local_2'
},
{
'Identifier': 'z_4',
'FileSystem': 'obs',
'Path': 'mno.zarr',
'StoreInstanceId': 'obs_1'
},
{
'Identifier': 'z_2',
'FileSystem': 'local',
'Path': 'ghi.zarr',
'StoreInstanceId': 'local_1'
},
{
'Identifier': 'z_3',
'FileSystem': 'local',
'Path': 'more/path/jkl.zarr',
'StoreInstanceId': 'local_2'
},
{
'Identifier': 'z_5',
'FileSystem': 'obs',
'Path': 'pqr.zarr',
'StoreInstanceId': 'obs_1'
},
{
'Identifier': 'z_6',
'FileSystem': 'local',
'Path': 'path/stu.zarr',
'StoreInstanceId': 'local_2'
},
{
'Identifier': 'z_7',
'FileSystem': 'local',
'Path': 'more/path/vwx.zarr',
'StoreInstanceId': 'local_2'
},
]
self.assertEqual(expected_dataset_configs, adjusted_dataset_configs)

def test_with_instance_id(self):
ctx = new_test_service_context()
dataset_config = {'Identifier': 'zero',
'Title': 'Test 0',
'FileSystem': 'local',
'StoreInstanceId': 'some_id'}
dataset_config_copy = dataset_config.copy()

ctx.config['Datasets'] = [dataset_config]
dataset_config = ctx.get_dataset_configs()[0]

self.assertEqual(dataset_config_copy, dataset_config)

def test_local(self):
ctx = new_test_service_context()
dataset_config = {'Identifier': 'one',
'Title': 'Test 1',
'FileSystem': 'local',
'Path': 'cube-1-250-250.zarr'}

ctx.config['Datasets'] = [dataset_config]
dataset_config = ctx.get_dataset_configs()[0]

self.assertEqual(['Identifier', 'Title', 'FileSystem', 'Path',
'StoreInstanceId'],
list(dataset_config.keys()))
self.assertEqual('one',
dataset_config['Identifier'])
self.assertEqual('Test 1', dataset_config['Title'])
self.assertEqual('local', dataset_config['FileSystem'])
self.assertEqual('cube-1-250-250.zarr', dataset_config["Path"])
self.assertEqual('local_1', dataset_config['StoreInstanceId'])

def test_s3(self):
ctx = new_test_service_context()
dataset_config = {'Identifier': 'two',
'Title': 'Test 2',
'FileSystem': 'obs',
'Endpoint': 'https://s3.eu-central-1.amazonaws.com',
'Path': 'xcube-examples/OLCI-SNS-RAW-CUBE-2.zarr',
'Region': 'eu-central-1'}

ctx.config['Datasets'] = [dataset_config]
dataset_config = ctx.get_dataset_configs()[0]

self.assertEqual(['Identifier', 'Title', 'FileSystem', 'Endpoint',
'Path', 'Region', 'StoreInstanceId'],
list(dataset_config.keys()))
self.assertEqual('two', dataset_config['Identifier'])
self.assertEqual('Test 2', dataset_config['Title'])
self.assertEqual('obs', dataset_config['FileSystem'])
self.assertEqual('https://s3.eu-central-1.amazonaws.com',
dataset_config['Endpoint'])
self.assertEqual('OLCI-SNS-RAW-CUBE-2.zarr', dataset_config['Path'])
self.assertEqual('eu-central-1', dataset_config['Region'])
self.assertEqual('obs_1', dataset_config['StoreInstanceId'])

def test_memory(self):
ctx = new_test_service_context()
dataset_config = {'Identifier': 'three',
'Title': 'Test 3',
'FileSystem': 'memory'}
dataset_config_copy = dataset_config.copy()

ctx.config['Datasets'] = [dataset_config]
dataset_config = ctx.get_dataset_configs()[0]

self.assertEqual(dataset_config_copy, dataset_config)

def test_missing_file_system(self):
ctx = new_test_service_context()
dataset_config = {'Identifier': 'five',
'Title': 'Test 5',
'Path': 'cube-1-250-250.zarr'}

ctx.config['Datasets'] = [dataset_config]
dataset_config = ctx.get_dataset_configs()[0]

self.assertEqual(['Identifier', 'Title', 'Path', 'StoreInstanceId'],
list(dataset_config.keys()))
self.assertEqual('five', dataset_config['Identifier'])
self.assertEqual('Test 5', dataset_config['Title'])
self.assertEqual('cube-1-250-250.zarr', dataset_config['Path'])
self.assertEqual('local_1', dataset_config['StoreInstanceId'])

def test_local_store_already_existing(self):
ctx = new_test_service_context()
dataset_config_1 = {'Identifier': 'six',
'Title': 'Test 6',
'FileSystem': 'local',
'Path': 'cube-1-250-250.zarr'}
dataset_config_2 = {'Identifier': 'six_a',
'Title': 'Test 6 a',
'FileSystem': 'local',
'Path': 'cube-5-100-200.zarr'}

ctx.config['Datasets'] = [dataset_config_1, dataset_config_2]
dataset_configs = ctx.get_dataset_configs()

self.assertEqual(dataset_configs[0]['StoreInstanceId'],
dataset_configs[1]['StoreInstanceId'])

def test_s3_store_already_existing(self):
ctx = new_test_service_context()
dataset_config_1 = {'Identifier': 'seven',
'Title': 'Test 7',
'FileSystem': 'obs',
'Endpoint': 'https://s3.eu-central-1.amazonaws.com',
'Path': 'xcube-examples/OLCI-SNS-RAW-CUBE-2.zarr',
'Region': 'eu-central-1'}

dataset_config_2 = {'Identifier': 'seven_a',
'Title': 'Test 7 a',
'FileSystem': 'obs',
'Endpoint': 'https://s3.eu-central-1.amazonaws.com',
'Path': 'xcube-examples/OLCI-SNS-RAW-CUBE-3.zarr',
'Region': 'eu-central-1'}

ctx.config['Datasets'] = [dataset_config_1, dataset_config_2]
dataset_configs = ctx.get_dataset_configs()

self.assertEqual(dataset_configs[0]['StoreInstanceId'],
dataset_configs[1]['StoreInstanceId'])
22 changes: 21 additions & 1 deletion xcube/core/store/storepool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,27 @@ def store_instance_ids(self) -> List[str]:
def store_configs(self) -> List[DataStoreConfig]:
return [v.store_config for k, v in self._instances.items()]

def has_store_config(self, store_instance_id: str) -> bool:
def get_store_instance_id(self,
store_config: DataStoreConfig,
strict_check: bool = False
) \
-> Optional[str]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style:

Suggested change
def get_store_instance_id(self,
store_config: DataStoreConfig,
strict_check: bool = False
) \
-> Optional[str]:
def get_store_instance_id(self,
store_config: DataStoreConfig,
strict_check: bool = False
) \
-> Optional[str]:
Suggested change
def get_store_instance_id(self,
store_config: DataStoreConfig,
strict_check: bool = False
) \
-> Optional[str]:
def get_store_instance_id(
self,
store_config: DataStoreConfig,
strict_check: bool = False
) -> Optional[str]:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub has a bug here: it doubles the suggestions! Only the second is valid!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It even doubles every comment!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wanted to check our code style for this issue. I did not find it, I just recall we already talked about this before.

But are you certain that the suggestion below is right? Going to the lowest indention level with the method header?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not find it, I just recall we already talked about this before.

PEP-8. And yes, we talked about this before, see https://www.python.org/dev/peps/pep-0008/#indentation

Going to the lowest indention level with the method header?

No that's not right, it happened per accident.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, changed it according to pep8

assert_instance(store_config, DataStoreConfig, 'store_config')
for id, instance in self._instances.items():
if strict_check:
if instance.store_config == store_config:
return id
else:
if instance.store_config.store_id == store_config.store_id and \
instance.store_config.store_params == \
store_config.store_params:
return id
return None

def has_store_config(self, store_config: DataStoreConfig) -> bool:
return self.get_store_instance_id(store_config) is not None

def has_store_instance(self, store_instance_id: str) -> bool:
assert_instance(store_instance_id, str, 'store_instance_id')
return store_instance_id in self._instances

Expand Down
4 changes: 3 additions & 1 deletion xcube/webapi/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ def get_schema(cls) -> JsonObjectSchema:
factory=DatasetConfig,
required=[
'Identifier',
'Path'
],
properties=dict(
Identifier=IdentifierSchema,
Expand Down Expand Up @@ -181,10 +182,11 @@ def get_schema(cls) -> JsonObjectSchema:
return JsonObjectSchema(
factory=DataStoreDatasetConfig,
required=[
'Identifier'
'Path'
],
properties=dict(
Identifier=IdentifierSchema,
Path=PathSchema,
StoreInstanceId=IdentifierSchema, # will be set by server
StoreOpenParams=JsonObjectSchema(additional_properties=True),
**_get_common_dataset_properties()
Expand Down
Loading