implement 'list_projects' with the org access feature #825
Conversation
alexey-cord-tech
requested review from
adam-encord,
sergei-encord and
Jim-Encord
Unit test report ((Pydantic 2.x)193 tests 193 ✅ 6s ⏱️ Results for commit 24da0e4. ♻️ This comment has been updated with latest results. |
Unit test report (Pydantic 1.x)193 tests 193 ✅ 6s ⏱️ Results for commit 24da0e4. ♻️ This comment has been updated with latest results. |
| @@ -1139,6 +1139,7 @@ def list_project_datasets(self, project_hash: UUID) -> Iterable[ProjectDataset]: | |||
| .results | |||
| ) | |||
|
|
|||
| @deprecated("0.1.102", alternative="encord.ontology.Ontology class") | |||
encord/project.py
Outdated
| @@ -117,21 +118,38 @@ def ontology(self) -> Dict[str, Any]: | |||
| This method returns the same structure as :meth:`encord.Project.ontology_structure`, just in | |||
| raw python dictionary format. | |||
| """ | |||
| return self._ontology.structure.to_dict() | |||
| return self._ontology_instance.structure.to_dict() | |||
There was a problem hiding this comment.
In a non-ideal scenario when the customer relies on _ontology, they'll get exception when trying to call methods from None object. I guess we should expose _ontology property and rename the private attribute to something like __lazy_load_ontology so at least it wouldn't break crazy use cases
There was a problem hiding this comment.
This is a fair point but I think that we can probably change the private internals without it being too much of an issue. Have we checked that all of our private code doesn't use _ontology directly? Need to ensure that we don't attempt to actually use it in label parsing for example and we haven't initialised it?
E.G: Can we ensure that we have always fetched the Ontology before we attempt to parse label blobs?
There was a problem hiding this comment.
I think it's totally fair to break backwards compat in underscore-prefix parts.
There was a problem hiding this comment.
+1, that's not a part of the public interface, so no problem here.
And this method is deprecated for more than a year anyways.
| yield Project( | ||
| client=client, | ||
| project_instance=row, | ||
| ontology=None, # lazy-load |
There was a problem hiding this comment.
Damn, crazy approach. Is it worth writing a test checking that this method does atmost 2-3 network calls and codify this solution?
There was a problem hiding this comment.
Well on dev the list_projects returns a few tens of thousand items ;), so with a non-lazy version this test took forever. So we kind of have this by accident.
We can probably make this a unit test, but it's not super easy to write...
encord/project.py
Outdated
| @@ -117,21 +118,38 @@ def ontology(self) -> Dict[str, Any]: | |||
| This method returns the same structure as :meth:`encord.Project.ontology_structure`, just in | |||
| raw python dictionary format. | |||
| """ | |||
| return self._ontology.structure.to_dict() | |||
| return self._ontology_instance.structure.to_dict() | |||
There was a problem hiding this comment.
This is a fair point but I think that we can probably change the private internals without it being too much of an issue. Have we checked that all of our private code doesn't use _ontology directly? Need to ensure that we don't attempt to actually use it in label parsing for example and we haven't initialised it?
E.G: Can we ensure that we have always fetched the Ontology before we attempt to parse label blobs?
Introduction and Explanation
Next step towards org-wide access: projects.
Unfortunately we're "doing too much" in project listing, and doing this within the confines of the old
get_projectswas not feasible (would have to fetch a lot of data potentially, with the label rows and editors and stuff).So I'm adding the new
list_projectsinstead, with a nicer API; also doing lots of unrelated-ish refactorings/cleanups because I initially attempted to do it backwards-compatibly.Documentation
Docstrings; the main docs to be written
Tests
Coming in the BE PR https://github.com/encord-team/cord-backend/pull/4431
Known issues
This is so hairy and took me 10x the estimated time :/