Implement experimental release of replication ops #1673
Conversation
![orca-security-eu[bot]](https://avatars.githubusercontent.com/in/276244?s=60&v=4){kind=small}
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
 Passed |
Secrets |  0  0  0  0 |
View in Orca |
| self, | ||
| *, | ||
| collection: str, | ||
| shard: Optional[str], |
There was a problem hiding this comment.
should this have the default None?
There was a problem hiding this comment.
Ah, because of the specific pattern I'm adding in weaviate.replication, it doesn't use the stubs-auto-gen logic instead spelling out the async_.py and sync.py files explicitly, the defaults go in those files with the executor not permitting them. I will remove the default from transfer_type above to make it clearer
weaviate/replication/models.py
Outdated
|
|
||
|
|
||
| class _ReplicationReplicateDetailsReplicaStatus(TypedDict): | ||
| state: Literal["REGISTERED", "HYDRATING", "FINALIZING", "DEHYDRATING", "READY", "CANCELLED"] |
There was a problem hiding this comment.
can't you just use the enum as type here?
There was a problem hiding this comment.
These are just the types of the API response so are the types of response.json(). The Literal here is for internal use only to avoid serialization. Turning them into enums would just move any error on incompatible types from the public class to this private one
There was a problem hiding this comment.
It is more about avoiding duplication - I am sure there will be more states added and then you have to add it in two places (which will be forgotten)
There was a problem hiding this comment.
Ideally, these TypedDict classes would be auto-generated from the schema.json file, but I don't think such a thing exists. The duplication is a good point, I will simply remove these TypedDict classes and have the serialisation work on raw dict. Either way, it needs runtime tests to ensure correctness anyway despite pyright
|
Great to see you again! Thanks for the contribution. |
|
Hey QQ: does it make sense for this to be under |
@databyjp I think this is a good idea since that is what it is most relevant to |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev/1.31 #1673 +/- ##
===========================================
Coverage ? 87.43%
===========================================
Files ? 266
Lines ? 18139
Branches ? 0
===========================================
Hits ? 15859
Misses ? 2280
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tsmith023
changed the title
client.replication namespace for all replicate ops
|
Closing in favour of #1708 |
No description provided.