Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DBWorkflow's index gets out of sync #1137

Closed
remram44 opened this issue Oct 23, 2015 · 3 comments
Closed

DBWorkflow's index gets out of sync #1137

remram44 opened this issue Oct 23, 2015 · 3 comments

Comments

@remram44
Copy link
Member

I have no idea why any of this is there of how it work. It seems to me that the DB layer was meant to be an easy-to-use, efficient object storage, but it is nothing like that [1].

The problem this time is that Pipeline#clear() does not clear the DBWorkflow's "index" thing, the objects dictionary (because db_delete_module() doesn't), so objects stay in there. I lost most of today tracking that down while working the interpreter...


[1]: The piece of code for deleting a module loops until it finds it!

    def db_delete_module(self, module):
        self.is_dirty = True
        for i in xrange(len(self._db_modules)):
            if self._db_modules[i].db_id == module.db_id:
                ...
@dakoop
Copy link
Member

dakoop commented Oct 24, 2015

Yes, the db code would benefit from revision. The original goal was to provide serialization to XML and RDBMS (and potentially other formats). In order to preserve legacy access, various indexes were added but order was also mattered for some features where only that was used and ids didn't originally exist. Basically, we wanted to add the db code without changing the logic at the core level. This, and my beginning Python knowledge at the time, caused code like the ugly list/dictionary code above.

@dakoop
Copy link
Member

dakoop commented Nov 4, 2015

Looks like db_delete_module shouldn't be called but rather db_delete_object in Pipeline.clear.

@remram44
Copy link
Member Author

remram44 commented Nov 5, 2015

Should be fixed by c24e310. Thanks for the help @dakoop!

@remram44 remram44 closed this as completed Nov 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants