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

Stop inserting data into parent tables #1

Closed
wants to merge 9 commits into from

Conversation

emord
Copy link

@emord emord commented Oct 2, 2018

@dimagi/scale-team This is a change list after merging maxtepkeev#45 into the branch that we run on prod. There is some background in maxtepkeev#26 and maxtepkeev#43 but the gist of it is the way architect works by default is to insert into the parent & child tables, return the id, delete from the parent table.

By default this does nothing differently. If we add the return_null option, this will change it to only inserting into the child table and not returning the ID. this can break in django if we try to use the returned record (i.e. you need to handle the primary key in python code), but I plan to use this for UCRs so we only need to worry about sqlalchemy

Copy link

@snopoke snopoke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't done a full review, let me know if you need me to.

@emord
Copy link
Author

emord commented Oct 9, 2018

This was pointed to by dimagi/commcare-hq#21994 which is now merged. I also rebuilt the current triggers so that they are inserting directly to the child tables.

We're still using a tag because I don't think either of these two PRs will get merged upstream. I think depending on the citus stuff we may end up removing this dependency anyways, so I'm going to keep this as pointing to a tag. If we still need this afterwards I will come up with a plan to get this properly versioned. Going to put it in the tech debt trello https://trello.com/c/MxJoiRaV/108-path-forward-for-architect

@emord emord closed this Oct 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants