Assign instance IDs in the Supervisor [ECR-3746] #1552
Conversation
Codecov Report
@@ Coverage Diff @@
## dynamic_services #1552 +/- ##
====================================================
+ Coverage 93.81% 93.82% +<.01%
====================================================
Files 203 203
Lines 29498 29494 -4
====================================================
- Hits 27673 27672 -1
+ Misses 1825 1822 -3
Continue to review full report at Codecov.
|
| let id = self | ||
| .vacant_instance_id | ||
| .get() | ||
| .unwrap_or(MAX_BUILTIN_INSTANCE_ID); |
There was a problem hiding this comment.
What about to rename to FIRST_DEPLOYED_INSTANCE_ID ? As I can see builtin instance can't get id 1024.
There was a problem hiding this comment.
It'll get removed soon anyway, so I don't think it matters (see ECR-3851) :)
Co-Authored-By: Oleksandr Anyshchenko <oleksandr.anyshchenko@xdev.re>
| access: &Prefixed<'a, T>, | ||
| index_name: &str, | ||
| ) -> U { | ||
| FromAccess::from_access(access.clone(), index_name.into()).unwrap() |
There was a problem hiding this comment.
I wonder if this unwrap is really safe?
There was a problem hiding this comment.
Error is returned if the index exists but has a different type. Since this function is private and used only in the schema constructor (thus types are correct by design), this unwrap is indeed safe.
This PR moves instance ID assignment logic from Dispatcher to the Supervisor.
I decided to not remove
MAX_BUILTIN_INSTANCE_IDconstant in this PR, but created a task in Jira for it (ECR-3851).