1
\$\begingroup\$

I am inserting data from a dict type object into three different tables in the SQL database. I am using Sqlalchemy as my ORM. My code is working but I think it is not very readable, is it any way to make it more readable or maybe simpler? I am using a lot of try except because I don't want my code to fail in the middle and also because all the three tables are related to each other.

My code is below:

def upload_fpb_data(data, project_id):
    fpb_count = len(data)
    fpb_added_count = 0
    fpb_children_count = sum(map(lambda x : x['count'], data))
    fpb_children_added_count = 0
    fpb_children_base_color_count = sum([sum(map(lambda x: x['count'], i['children'])) for i in data])
    fpb_children_base_color_added_count = 0
    for parent in data:
        parent_data = parent['data']
        add_fpb = FPB(stockCode=parent_data[0], description=parent_data[1], 
                        longDescription=parent_data[2], altKey=parent_data[3],
                        project=project_id)
        db.session.add(add_fpb)
        try:
            db.session.commit()
            fpb_added_count += 1
            for child1 in parent['children']:
                child1_data = child1['data']
                add_fpb_children = FpbChildrenAssociation(ParentPart=child1_data['ParentPart'],
                Component=child1_data['Component'], qty=child1_data['ComponentQtyPer'])
                db.session.add(add_fpb_children)
                
                try:
                    db.session.commit()
                    fpb_children_added_count+=1
                    fpb_children_association_id = add_fpb_children.id
                    for child2 in child1['children']['data']:
                        add_fpb_children_base_colors = FpbChildrenBaseColorAssociation(
                                fpbChildrenAssociationId=fpb_children_association_id,
                                baseColor=child2,
                            )
                        db.session.add(add_fpb_children_base_colors)
                        try:
                            db.session.commit()
                            fpb_children_base_color_added_count +=1
                        except:
                            db.session.rollback()
                    
                except Exception as e:
                    db.session.rollback()
            
        except Exception as e:
            db.session.rollback()
    return {'fpb':f"Rows Appended {fpb_added_count}/{fpb_count}",
            'fpbChildrenAssociation':f"Rows Appended {fpb_children_added_count}/{fpb_children_count}",
            'fpbChildrenBaseColorAssociation':f"Rows Appended {fpb_children_base_color_added_count}/{fpb_children_base_color_count}"}

if you want to see the data I am working with then here it is:

data = [{'table': 'fpb',
 'data': ('XPB.20.001.07.200.01', 'SSLDX STERN BOX', '', '20SX200'),
 'children': [{'table': 'fpbChildrenAssociation',
   'data': ['XPB.20.001.07.200.01', 'DID-1828A-50-V1.20.001.00', 1.0],
   'count': 3,
   'children': {'table': 'fpbChildrenBaseColorAssociation',
    'data': ['00', '60', '90']}},
  {'table': 'fpbChildrenAssociation',
   'data': ['XPB.20.001.07.200.01', 'RFH0136SLV', 1.0],
   'count': 3,
   'children': {'table': 'fpbChildrenBaseColorAssociation',
    'data': ['00', '60', '90']}},
  {'table': 'fpbChildrenAssociation',
   'data': ['XPB.20.001.07.200.01', 'RFN0019', 4.0],
   'count': 3,
   'children': {'table': 'fpbChildrenBaseColorAssociation',
    'data': ['00', '60', '90']}},
  {'table': 'fpbChildrenAssociation',
   'data': ['XPB.20.001.07.200.01', 'RFN0036', 4.0],
   'count': 3,
   'children': {'table': 'fpbChildrenBaseColorAssociation',
    'data': ['00', '60', '90']}},
  {'table': 'fpbChildrenAssociation',
   'data': ['XPB.20.001.07.200.01', 'RFN0043', 4.0],
   'count': 3,
   'children': {'table': 'fpbChildrenBaseColorAssociation',
    'data': ['00', '60', '90']}},
  {'table': 'fpbChildrenAssociation',
   'data': ['XPB.20.001.07.200.01', 'RFN0046B', 4.0],
   'count': 3,
   'children': {'table': 'fpbChildrenBaseColorAssociation',
    'data': ['00', '60', '90']}},
  {'table': 'fpbChildrenAssociation',
   'data': ['XPB.20.001.07.200.01', 'RFN0091', 4.0],
   'count': 3,
   'children': {'table': 'fpbChildrenBaseColorAssociation',
    'data': ['00', '60', '90']}},
  {'table': 'fpbChildrenAssociation',
   'data': ['XPB.20.001.07.200.01', 'RFN0126', 1.0],
   'count': 3,
   'children': {'table': 'fpbChildrenBaseColorAssociation',
    'data': ['00', '60', '90']}},
  {'table': 'fpbChildrenAssociation',
   'data': ['XPB.20.001.07.200.01', 'RPP0134', 4.0],
   'count': 3,
   'children': {'table': 'fpbChildrenBaseColorAssociation',
    'data': ['00', '60', '90']}},
  {'table': 'fpbChildrenAssociation',
   'data': ['XPB.20.001.07.200.01', 'X.001.07.44579', 1.0],
   'count': 1,
   'children': {'table': 'fpbChildrenBaseColorAssociation', 'data': ['60']}}],
 'count': 10}]
\$\endgroup\$
1
  • \$\begingroup\$ @BCdotWEB i have changed the title. Thanks \$\endgroup\$ Jun 29 at 7:23
1
\$\begingroup\$

If you are concerned about data integrity, that's what transactions are for. I don't really see the point of nested try/catch blocks. A single block for the whole procedure would be sufficient.

So, at the start of your try block, you start an explicit transaction using session.begin(), you do your stuff and finally you commit at the end, or rollback in case of exception. That's it.

As an example:

# verbose version of what a context manager will do
with Session(engine) as session:
    session.begin()
    try:
        session.add(some_object)
        session.add(some_other_object)
    except:
        session.rollback()
        raise
    else:
        session.commit()

The source (recommended): SQLAlchemy - Session Basics

If you want an all or nothing operation that's what you should be doing. Presently, you are doing regular commits, and it's perfectly possible that the code will fail in the middle. Thus some tables can be populated without matching records in the other related tables.

Just removing those nested blocks should increase readability. Then consider adding some comments. And line spacing. The code is a bit terse. Regarding the naming conventions, I am not sure about what kind of data you are handling but I am thinking the names could perhaps be more telling that child or children something ?

Instead of incrementing a counter manually like this:

fpb_added_count += 1

you could use the enumerate function like this:

for fpb_added_count, child1 in enumerate(parent['children'], start=1):

But your objects may have a len() property already right. Do you really need to keep track of item counts separately ?

This variable child1_data does not seem to be really useful:

child1_data = child1['data']

Why not simply just use child1['data'].

Same for fpb_children_association_id etc, that makes the code harder to follow for no real benefit I think. There is no real improvement or simplification.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for the answer. I am not looking to rollback the whole transaction if only one table fails. That's why I added them in a particular order. Do you there is an easier way to do that? Yes, I would like to keep the track of the counts so that I can tell the user how many of the rows have been added out of the actual number of rows for each table. \$\endgroup\$ Jun 29 at 2:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.