michwill on 17 Nov Member
Oh wow... Was it supposed to be in all the time?

vzotova on 18 Nov Author Member
I think in production it's impossible to have two different registries classes with the same content, but in test it was all the time.
Also interesting moment - locally I still have old behavior with ordering which "good" for tests, but one commit changed it for CircleCI

cygnusv 11 days ago Member
Instead of the class name we should use the registry type:

Suggested change 
        blake.update(self.__class__.__name__.encode())
        blake.update(self.REGISTRY_TYPE.encode())
We don't want the ID to be different depending if it's a LocalContractRegistry, a TemporaryContractRegistry or an InMemoryContractRegistry, even though all of them are contract registries.

vzotova 10 days ago Author Member
while tests we create two same registries Local and Memory and they have same ID, it leads to bugs depending on ordering in cache dictionary. I see this ID not for content but for instance

cygnusv 6 days ago Member
Well, but if this is a problem for tests, then we should monkey-patch it in tests then. I don't see impossible that in production we could have two registries with the same content but different classes (e.g., an InMemory registry persisted to a LocalRegistry)
@KPrasch what do you think?

vicky.zToday at 11:01 AM
I'm still thinking that comparing only content is design issue, because registry cache has id as key and instance as value, and will be two same keys for different instances,
I see two solutions - change cache structure somehow or use id with class
 
dnunezToday at 11:02 AM
ok, can you open an issue?
 
vicky.zToday at 11:03 AM
hmmm, I'm not sure what it should be, I mean id with class is simplest solution and it's already there
 
dnunezToday at 11:05 AM
well, I still don't see why an in-memory registry should be mapped differently than a file-based registry if they have the same data, which is what happens with this solution
 
vicky.zToday at 11:06 AM
I can provide example - if we have two same registries but in different instances, it's possible to change only one instance and they will be different
 
dnunezToday at 11:07 AM
ok....? what's wrong with that?
 
vicky.zToday at 11:09 AM
will be totally uncontrolled situation when we have two different registries, but agents that were created will think that only one registry, my point that if we want id -> content, then we should store content not instance
 
dnunezToday at 11:10 AM
ah, the issue is more related to the registry being mutable?
 
vicky.zToday at 11:12 AM
kind of, but not only this, we have cache as id -> agent_class -> agent, so issue when you try to get agent using instance1 but will return agent with instance2 because ids were equal
ContractAgency.get_agent(instance1) will return Agent(instance2), I see that as an issue, so we just want ContractAgency.get_agent(instance1) will return Agent(instance1)
 
dnunezToday at 11:15 AM
well, if content is the same, what's the problem? Agents are stateless anyway
 
vicky.zToday at 11:15 AM
problem from code perspective, it's bad practice when you get not what you want
 
vicky.zToday at 11:17 AM
It was really difficult to find out where is problem, because locally I can't reproduce it
so what if it will affect somehow production, I'm for transparent layers and logic
other possible solution: change cache to something like id -> registry_class -> agent_class -> agent