[r] Rename SourceName to SourceSpec (#2843) #3047
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amarjandu
force-pushed
the
issues/amar/2843-mv-spec
branch
3 times, most recently
from
ed15c81
to
511a32b
Compare
Codecov Report
@@ Coverage Diff @@
## develop #3047 +/- ##
===========================================
- Coverage 82.41% 82.26% -0.15%
===========================================
Files 118 119 +1
Lines 13288 13149 -139
===========================================
- Hits 10951 10817 -134
+ Misses 2337 2332 -5
Continue to review full report at Codecov.
|
amarjandu
force-pushed
the
issues/amar/2843-mv-spec
branch
from
511a32b
to
098fb01
Compare
amarjandu
force-pushed
the
issues/amar/2843-mv-spec
branch
3 times, most recently
from
ac36646
to
4353199
Compare
dsotirho-ucsc
requested changes
May 13, 2021
There was a problem hiding this comment.
Found a couple more spots.
diff --git a/scripts/recan_bundle_tdr.py b/scripts/recan_bundle_tdr.py
index 6ca3fead..9c616e7a 100644
--- a/scripts/recan_bundle_tdr.py
+++ b/scripts/recan_bundle_tdr.py
@@ -388,7 +388,7 @@ def main(argv):
dss_source = DSSSourceRef(id='',
spec=SimpleSourceSpec(prefix='',
- name=config.dss_endpoint))
+ spec=config.dss_endpoint))
dss_bundle = DSSBundle(fqid=SourcedBundleFQID(source=dss_source,
uuid=args.bundle_uuid,
version=''),
@@ -396,7 +396,7 @@ def main(argv):
metadata_files=metadata)
tdr_source = TDRSourceRef(id=args.source_id,
- name=TDRSourceSpec(project='test_project',
+ spec=TDRSourceSpec(project='test_project',
name='test_name',
is_snapshot=True))
tdr_bundle = dss_bundle_to_tdr(dss_bundle, tdr_source)
diff --git a/src/azul/azulclient.py b/src/azul/azulclient.py
index ed64b55a..efd1c3af 100644
--- a/src/azul/azulclient.py
+++ b/src/azul/azulclient.py
@@ -293,7 +293,7 @@ class AzulClient(object):
>>> AzulClient.filter_obsolete_bundle_versions([])
[]
>>> from azul.indexer import SimpleSourceSpec, SourceRef
- >>> s = SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', name='n'))
+ >>> s = SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', spec='n'))
>>> def b(u, v):
... return SourcedBundleFQID(source=s, uuid=u, version=v)
>>> AzulClient.filter_obsolete_bundle_versions([
@@ -303,32 +303,32 @@ class AzulClient(object):
... ]) # doctest: +NORMALIZE_WHITESPACE
[SourcedBundleFQID(uuid='c',
version='0',
- source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', name='n'))),
+ source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', spec='n'))),
SourcedBundleFQID(uuid='b',
version='3',
- source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', name='n'))),
+ source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', spec='n'))),
SourcedBundleFQID(uuid='a',
version='1',
- source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', name='n')))]
+ source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', spec='n')))]
>>> AzulClient.filter_obsolete_bundle_versions([
... b('C', '0'), b('a', '1'), b('a', '0'),
... b('a', '2'), b('b', '1'), b('c', '2')
... ]) # doctest: +NORMALIZE_WHITESPACE
[SourcedBundleFQID(uuid='c',
version='2',
- source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', name='n'))),
+ source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', spec='n'))),
SourcedBundleFQID(uuid='b',
version='1',
- source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', name='n'))),
+ source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', spec='n'))),
SourcedBundleFQID(uuid='a',
version='2',
- source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', name='n')))]
+ source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', spec='n')))]
>>> AzulClient.filter_obsolete_bundle_versions([
... b('a', '0'), b('A', '1')
... ]) # doctest: +NORMALIZE_WHITESPACE
[SourcedBundleFQID(uuid='A',
version='1',
- source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', name='n')))]
+ source=SourceRef(id='i', spec=SimpleSourceSpec(prefix='42', spec='n')))]
"""
# Sort lexicographically by source and FQID. I've observed the DSS
diff --git a/src/azul/indexer/__init__.py b/src/azul/indexer/__init__.py
index 8753d157..1e9aedbe 100644
--- a/src/azul/indexer/__init__.py
+++ b/src/azul/indexer/__init__.py
@@ -59,14 +59,14 @@ class SourceSpec(ABC, Generic[SOURCE_SPEC]):
The name of a repository source containing bundles to index. A repository
has at least one source. Repository plugins whose repository source names
are structured might want to implement this abstract class. Plugins that
- have simple unstructured names may want to use :class:`StringSourceName`.
+ have simple unstructured names may want to use :class:`SimpleSourceSpec`.
"""
prefix: Optional[str] = ''
@classmethod
@abstractmethod
- def parse(cls, name: str) -> SOURCE_SPEC:
+ def parse(cls, source: str) -> SOURCE_SPEC:
raise NotImplementedError
@abstractmethod
@@ -79,14 +79,14 @@ class SimpleSourceSpec(SourceSpec['SimpleSourceSpec']):
"""
Default implementation for unstructured source names.
"""
- name: str
+ spec: str
@classmethod
- def parse(cls, name: str) -> 'SimpleSourceSpec':
- return cls(name=name)
+ def parse(cls, spec: str) -> 'SimpleSourceSpec':
+ return cls(spec=spec)
def __str__(self) -> str:
- return self.name
+ return self.spec
SOURCE_REF = TypeVar('SOURCE_REF', bound='SourceRef')
@@ -133,8 +133,8 @@ class SourceRef(Generic[SOURCE_SPEC, SOURCE_REF]):
Traceback (most recent call last):
...
azul.RequirementError: ('Ambiguous source names for same ID.',
- SimpleSourceSpec(prefix='', name='a'),
- SimpleSourceSpec(prefix='', name='b'),
+ SimpleSourceSpec(prefix='', spec='a'),
+ SimpleSourceSpec(prefix='', v='b'),
'1')
Interning is done per class:
@@ -162,7 +162,7 @@ class SourceRef(Generic[SOURCE_SPEC, SOURCE_REF]):
return self
def to_json(self):
- return dict(id=self.id, name=str(self.spec))
+ return dict(id=self.id, spec=str(self.spec))
@attr.s(auto_attribs=True, frozen=True, kw_only=True, order=True)
diff --git a/src/azul/indexer/document.py b/src/azul/indexer/document.py
index 246fc2cb..9b2cc5ee 100644
--- a/src/azul/indexer/document.py
+++ b/src/azul/indexer/document.py
@@ -604,7 +604,7 @@ class DocumentSource(SourceRef[SimpleSourceSpec, SourceRef]):
@classmethod
def from_json(cls, source: JSON) -> 'DocumentSource':
- return cls(id=source['id'], spec=SimpleSourceSpec(name=source['name']))
+ return cls(id=source['id'], spec=SimpleSourceSpec(spec=source['spec']))
@dataclass
@@ -687,7 +687,7 @@ class Aggregate(Document[AggregateCoordinates]):
'num_contributions': pass_thru_int,
'sources': {
'id': pass_thru_str,
- 'name': pass_thru_str
+ 'spec': pass_thru_str
},
'bundles': {
'uuid': pass_thru_str,
@@ -707,7 +707,7 @@ class Aggregate(Document[AggregateCoordinates]):
return super().mandatory_source_fields() + [
'num_contributions',
'sources.id',
- 'sources.name',
+ 'sources.spec',
]
def to_json(self) -> JSON:
diff --git a/src/azul/indexer/index_controller.py b/src/azul/indexer/index_controller.py
index e728465c..ad9314e2 100644
--- a/src/azul/indexer/index_controller.py
+++ b/src/azul/indexer/index_controller.py
@@ -168,7 +168,7 @@ class IndexController:
"""
match, source = notification['match'], notification['source']
plugin = self.repository_plugin(catalog)
- source = plugin.resolve_source(spec=source['name'], id=source['id'])
+ source = plugin.resolve_source(spec=source['spec'], id=source['id'])
bundle_fqid = SourcedBundleFQID(source=source,
uuid=match['bundle_uuid'],
version=match['bundle_version'])
diff --git a/src/azul/plugins/metadata/hca/__init__.py b/src/azul/plugins/metadata/hca/__init__.py
index bfd85457..0ee0b547 100644
--- a/src/azul/plugins/metadata/hca/__init__.py
+++ b/src/azul/plugins/metadata/hca/__init__.py
@@ -200,7 +200,7 @@ class Plugin(MetadataPlugin):
manifest={
"sources": {
"source_id": "id",
- "source_name": "name",
+ "source_spec": "spec",
},
"bundles": {
"bundle_uuid": "uuid",
diff --git a/src/azul/plugins/repository/dss/__init__.py b/src/azul/plugins/repository/dss/__init__.py
index 815695b5..ed3c137d 100644
--- a/src/azul/plugins/repository/dss/__init__.py
+++ b/src/azul/plugins/repository/dss/__init__.py
@@ -80,7 +80,7 @@ class DSSSourceRef(SourceRef[SimpleSourceSpec, 'DSSSourceRef']):
# within a document, which is helpful for testing.
return cls(id=cls.id_from_name(endpoint),
spec=SimpleSourceSpec(prefix=config.dss_query_prefix,
- name=endpoint))
+ spec=endpoint))
@classmethod
def id_from_name(cls, name: str) -> str:
diff --git a/src/azul/service/hca_response_v5.py b/src/azul/service/hca_response_v5.py
index a02d57c4..1d615274 100644
--- a/src/azul/service/hca_response_v5.py
+++ b/src/azul/service/hca_response_v5.py
@@ -259,7 +259,7 @@ class KeywordSearchResponse(AbstractResponse, EntryFetcher):
def make_sources(self, entry):
return [
- {'sourceId': s['id'], 'sourceName': s['name']}
+ {'sourceId': s['id'], 'sourceName': s['spec']}
for s in entry['sources']
]
diff --git a/src/azul/terra.py b/src/azul/terra.py
index 3c5f8770..4ae50895 100644
--- a/src/azul/terra.py
+++ b/src/azul/terra.py
@@ -84,7 +84,7 @@ class TDRSourceSpec(SourceSpec):
_type_snapshot = 'snapshot'
@classmethod
- def parse(cls, source: str) -> 'TDRSourceSpec':
+ def parse(cls, spec: str) -> 'TDRSourceSpec':
"""
Construct an instance from its string representation, using the syntax
'tdr:{project}:{type}/{name}:{prefix}'.
@@ -121,7 +121,7 @@ class TDRSourceSpec(SourceSpec):
azul.uuids.InvalidUUIDPrefixError: 'n32' is not a valid UUID prefix.
"""
# BigQuery (and by extension the TDR) does not allow : or / in dataset names
- service, project, name, prefix = source.split(':')
+ service, project, name, prefix = spec.split(':')
type, name = name.split('/')
assert service == 'tdr', service
if type == cls._type_snapshot:
@@ -132,7 +132,7 @@ class TDRSourceSpec(SourceSpec):
assert False, type
validate_uuid_prefix(prefix)
self = cls(prefix=prefix, project=project, name=name, is_snapshot=is_snapshot)
- assert source == str(self), (source, self)
+ assert spec == str(self), (spec, self)
return self
@property
diff --git a/test/indexer/data/aaa96233-bf27-44c7-82df-b4dc15ad4d9d.2018-11-02T113344.698028Z.results.json b/test/indexer/data/aaa96233-bf27-44c7-82df-b4dc15ad4d9d.2018-11-02T113344.698028Z.results.json
index a8d9cf2d..bc3dbe7c 100644
--- a/test/indexer/data/aaa96233-bf27-44c7-82df-b4dc15ad4d9d.2018-11-02T113344.698028Z.results.json
+++ b/test/indexer/data/aaa96233-bf27-44c7-82df-b4dc15ad4d9d.2018-11-02T113344.698028Z.results.json
@@ -8,7 +8,7 @@
"entity_id": "aaa96233-bf27-44c7-82df-b4dc15ad4d9d",
"source": {
"id": "4b737739-4dc9-5d4b-9989-a4942047c91c",
- "name": "test"
+ "spec": "test"
},
"contents": {
"samples": [
@@ -463,7 +463,7 @@
"entity_id": "0c5ac7c0-817e-40d4-b1b1-34c3d5cfecdb",
"source": {
"id": "4b737739-4dc9-5d4b-9989-a4942047c91c",
- "name": "test"
+ "spec": "test"
},
"bundle_uuid": "aaa96233-bf27-44c7-82df-b4dc15ad4d9d",
"bundle_version": "2018-11-02T113344.698028Z",
@@ -696,7 +696,7 @@
"entity_id": "70d1af4a-82c8-478a-8960-e9028b3616ca",
"source": {
"id": "4b737739-4dc9-5d4b-9989-a4942047c91c",
- "name": "test"
+ "spec": "test"
},
"bundle_uuid": "aaa96233-bf27-44c7-82df-b4dc15ad4d9d",
"bundle_version": "2018-11-02T113344.698028Z",
@@ -929,7 +929,7 @@
"entity_id": "a21dc760-a500-4236-bcff-da34a0e873d2",
"source": {
"id": "4b737739-4dc9-5d4b-9989-a4942047c91c",
- "name": "test"
+ "spec": "test"
},
"bundle_uuid": "aaa96233-bf27-44c7-82df-b4dc15ad4d9d",
"bundle_version": "2018-11-02T113344.698028Z",
@@ -1186,7 +1186,7 @@
"entity_id": "e8642221-4c2c-4fd7-b926-a68bce363c88",
"source": {
"id": "4b737739-4dc9-5d4b-9989-a4942047c91c",
- "name": "test"
+ "spec": "test"
},
"bundle_uuid": "aaa96233-bf27-44c7-82df-b4dc15ad4d9d",
"bundle_version": "2018-11-02T113344.698028Z",
@@ -1446,7 +1446,7 @@
"sources": [
{
"id": "4b737739-4dc9-5d4b-9989-a4942047c91c",
- "name": "test"
+ "spec": "test"
}
],
"contents": {
@@ -1960,7 +1960,7 @@
"sources": [
{
"id": "4b737739-4dc9-5d4b-9989-a4942047c91c",
- "name": "test"
+ "spec": "test"
}
],
"contents": {
@@ -2252,7 +2252,7 @@
"sources": [
{
"id": "4b737739-4dc9-5d4b-9989-a4942047c91c",
- "name": "test"
+ "spec": "test"
}
],
"contents": {
@@ -2544,7 +2544,7 @@
"sources": [
{
"id": "4b737739-4dc9-5d4b-9989-a4942047c91c",
- "name": "test"
+ "spec": "test"
}
],
"contents": {
@@ -2794,7 +2794,7 @@
"sources": [
{
"id": "4b737739-4dc9-5d4b-9989-a4942047c91c",
- "name": "test"
+ "spec": "test"
}
],
"contents": {
@@ -3105,7 +3105,7 @@
"entity_id": "412898c5-5b9b-4907-b07c-e9b89666e204",
"source": {
"id": "4b737739-4dc9-5d4b-9989-a4942047c91c",
- "name": "test"
+ "spec": "test"
},
"contents": {
"samples": [
@@ -3363,7 +3363,7 @@
"sources": [
{
"id": "4b737739-4dc9-5d4b-9989-a4942047c91c",
- "name": "test"
+ "spec": "test"
}
],
"contents": {
diff --git a/test/service/test_manifest.py b/test/service/test_manifest.py
index 1d4ad36d..4c3f7a1e 100644
--- a/test/service/test_manifest.py
+++ b/test/service/test_manifest.py
@@ -185,7 +185,7 @@ class TestManifestEndpoints(ManifestTestCase, DSSUnitTestCase):
def test_manifest(self):
expected = [
('source_id', '4b737739-4dc9-5d4b-9989-a4942047c91c', '4b737739-4dc9-5d4b-9989-a4942047c91c'),
- ('source_name', 'test', 'test'),
+ ('source_spec', 'test', 'test'),
('bundle_uuid', 'f79257a7-dfc6-46d6-ae00-ba4b25313c10', 'f79257a7-dfc6-46d6-ae00-ba4b25313c10'),
('bundle_version', '2018-09-14T133314.453337Z', '2018-09-14T133314.453337Z'),
('file_document_id', '89e313db-4423-4d53-b17e-164949acfa8f', '6c946b6c-040e-45cc-9114-a8b1454c8d20'),
@@ -518,7 +518,7 @@ class TestManifestEndpoints(ManifestTestCase, DSSUnitTestCase):
'bundle_uuid': '587d74b4-1075-4bbf-b96a-4d1ede0481b2',
'bundle_version': '2018-09-14T133314.453337Z',
'source_id': '4b737739-4dc9-5d4b-9989-a4942047c91c',
- 'source_name': 'test',
+ 'source_spec': 'test',
'cell_suspension__provenance__document_id': '377f2f5a-4a45-4c62-8fb0-db9ef33f5cf0',
'cell_suspension__biomaterial_core__biomaterial_id': 'Q4_DEMO-cellsus_SAMN02797092',
'cell_suspension__estimated_cell_count': '',
@@ -615,7 +615,7 @@ class TestManifestEndpoints(ManifestTestCase, DSSUnitTestCase):
'bundle_uuid': 'aaa96233-bf27-44c7-82df-b4dc15ad4d9d',
'bundle_version': '2018-11-02T113344.698028Z',
'source_id': '4b737739-4dc9-5d4b-9989-a4942047c91c',
- 'source_name': 'test',
+ 'source_spec': 'test',
'cell_suspension__provenance__document_id': '412898c5-5b9b-4907-b07c-e9b89666e204',
'cell_suspension__biomaterial_core__biomaterial_id': 'GSM2172585 1',
'cell_suspension__estimated_cell_count': '1',
@@ -729,7 +729,7 @@ class TestManifestEndpoints(ManifestTestCase, DSSUnitTestCase):
'bundle_uuid',
'bundle_version',
'source_id',
- 'source_name',
+ 'source_spec',
'cell_suspension__provenance__document_id',
'cell_suspension__biomaterial_core__biomaterial_id',
'cell_suspension__estimated_cell_count',
|
@hannes-ucsc from your original patch #2650 (comment) you requested that If changing the attribute of |
|
When a variable or attribute references a value that is an instance of We're doing this to move away from |
amarjandu
force-pushed
the
issues/amar/2843-mv-spec
branch
3 times, most recently
from
3a755be
to
0608a72
Compare
dsotirho-ucsc
approved these changes
May 19, 2021
src/azul/indexer/__init__.py
Outdated
| """ | ||
| The name of a repository source containing bundles to index. A repository | ||
| has at least one source. Repository plugins whose repository source names | ||
| are structured might want to implement this abstract class. Plugins that | ||
| have simple unstructured names may want to use :class:`StringSourceName`. | ||
| have simple unstructured names may want to use :class:`StringSourceSpec`. |
There was a problem hiding this comment.
Suggested change
| have simple unstructured names may want to use :class:`StringSourceSpec`. | |
| have simple unstructured names may want to use :class:`SimpleSourceSpec`. |
There is no StringSourceSpec.
Been testing things out this morning, so far we look good... shall confirm before requesting primary review. edit* |
|
@hannes-ucsc The sandbox still has We can also patch/modify the results of the canned bundled to match the SourceSpec of each deployment. Might be a good time to bring up #2667, with a parent deployment being shared with the sandbox, this might have been caught earlier. |
|
Pulling this, need to update to include CannedStagingArea plugin as well... |
amarjandu
force-pushed
the
issues/amar/2843-mv-spec
branch
from
d256d59
to
b586d03
Compare
|
@hannes-ucsc please note the additional changes to the canned repository plugin. |
hannes-ucsc
requested changes
Jun 16, 2021
amarjandu
force-pushed
the
issues/amar/2843-mv-spec
branch
from
b586d03
to
547413e
Compare
hannes-ucsc
approved these changes
Jun 16, 2021
| @@ -821,7 +821,7 @@ def test_projects_key_search_response(self): | |||
| ], | |||
| "sources": [{ | |||
| "sourceId": "4b737739-4dc9-5d4b-9989-a4942047c91c", | |||
| "sourceName": "test" | |||
| "sourceSpec": "test:" | |||
jessebrennan
force-pushed
the
issues/amar/2843-mv-spec
branch
from
547413e
to
5c8e396
Compare
jessebrennan
added
the
sandbox
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Author
Author (reindex)
rtag to commit title or this PR does not require reindexingreindexlabel to PR or this PR does not require reindexingAuthor (freebies & chains)
chainlabel to the blocking PR or this PR is not chained to another PRAuthor (upgrading)
utag to commit title or this PR does not require upgradingupgradelabel to PR or this PR does not require upgradingAuthor (requirements, before every review)
make requirements_updateor this PR leaves requirements*.txt, common.mk and Makefile untouchedRtag to commit title or this PR leaves requirements*.txt untouchedreqslabel to PR or this PR leaves requirements*.txt untouchedAuthor (before every review)
make integration_testpasses in personal deployment or this PR does not touch functionality that could break the ITdevelop, squashed old fixupsPrimary reviewer (after approval)
no demono sandboxOperator (before pushing merge the commit)
reindexlabel andrcommit title tagno demono sandboxsandboxlabel or PR is labeledno sandboxsandboxor this PR does not require reindexingsandboxsandboxor this PR does not require reindexingsandboxOperator (after pushing the merge commit)
N reviewslabelling is accurateOperator (reindex)
devor this PR does not require reindexing or does not targetdevdevor this PR does not require reindexing or does not targetdevprodor this PR does not require reindexing or does not targetprodprodor this PR does not require reindexing or does not targetprodOperator