Remove extra if block and change more queries to use select

ros/api/v1/hosts.py

@@ -138,6 +138,7 @@ def get(self):
             .filter(PerformanceProfile.system_id.in_(system_query))
             .order_by(*sort_expression)
         )
+
         count = query.count()
         # NOTE: Override limit value to get all the systems when it is -1
         if limit == -1:
@@ -297,7 +298,7 @@ def get(self, host_id):
             RecommendationRating.rated_by == username
         ).first()
 
-        system = db.session.query(System).filter(System.inventory_id == host_id).first()
+        system = db.session.scalar(db.select(System).filter(System.inventory_id == host_id))
 
         record = None
         if profile:

ros/api/v1/recommendations.py

@@ -21,9 +21,8 @@
 
 
 class RecommendationsApi(Resource):
-
     recommendation_fields = {
-        'rule_id':  fields.String,
+        'rule_id': fields.String,
         'description': fields.String,
         'reason': fields.String,
         'resolution': fields.String,
@@ -59,10 +58,6 @@ def get(self, host_id):
         except exc.NoResultFound:
             abort(404, message=f"System {host_id} doesn't exist.")
 
-        if not system:
-            abort(404, message="host with id {} doesn't exist"
-                  .format(host_id))
-
         profile = PerformanceProfile.query.filter_by(system_id=system.id).first()
         if not profile:
             abort(
@@ -75,10 +70,10 @@ def get(self, host_id):
         if rule_hits:
             for rule_hit in rule_hits:
                 if filter_description:
-                    rule_data = db.session.query(Rule).filter(Rule.rule_id == rule_hit['rule_id'])\
-                                .filter(Rule.description.ilike(f'%{filter_description}%')).first()
+                    rule_data = db.session.scalar(db.select(Rule).filter(Rule.rule_id == rule_hit['rule_id'])
+                                                  .filter(Rule.description.ilike(f'%{filter_description}%')))
                 else:
-                    rule_data = db.session.query(Rule).filter(Rule.rule_id == rule_hit['rule_id']).first()
+                    rule_data = db.session.scalar(db.select(Rule).filter(Rule.rule_id == rule_hit['rule_id']))
                 if rule_data:
                     rule_dict = rule_data.__dict__
                     if system.cloud_provider is None:
@@ -95,7 +90,7 @@ def get(self, host_id):
                     if rule_hit.get("key") == 'INSTANCE_IDLE':
                         summaries = None
                     current_instance = f'{rule_hit_details.get("instance_type")} ' + \
-                        f'({rule_hit_details.get("price")} {INSTANCE_PRICE_UNIT})'
+                                       f'({rule_hit_details.get("price")} {INSTANCE_PRICE_UNIT})'
                     newline = '\n'
                     for skey in rules_columns:
                         formatted_candidates = []
@@ -108,7 +103,7 @@ def get(self, host_id):
                         recommendation[skey] = eval("f'{}'".format(rule_dict[skey]))
                     recommendations_list.append(recommendation)
         return {
-                  'inventory_id': system.inventory_id,
-                  'data': recommendations_list,
-                  'meta': {'count': len(recommendations_list)}
-            }
+            'inventory_id': system.inventory_id,
+            'data': recommendations_list,
+            'meta': {'count': len(recommendations_list)}
+        }

ros/lib/utils.py

@@ -37,7 +37,7 @@ def get_or_create(session, model, keys, **kwargs):
         keys = [keys]
     if not isinstance(keys, list):
         raise TypeError('keys argument must be a list or string')
-    instance = session.query(model).filter_by(**{k: kwargs[k] for k in keys}).first()
+    instance = session.scalars(db.select(model).filter_by(**{k: kwargs[k] for k in keys})).first()
     if instance:
         for k, v in kwargs.items():
             setattr(instance, k, v)
@@ -52,7 +52,7 @@ def update_system_record(session, **kwargs):
     inventory_id = kwargs.get('inventory_id')
     if inventory_id is None:
         return
-    instance = session.query(System).filter_by(inventory_id=inventory_id).first()
+    instance = session.scalars(db.select(System).filter_by(inventory_id=inventory_id)).first()
     if instance:
         for k, v in kwargs.items():
             setattr(instance, k, v)
@@ -62,7 +62,7 @@ def update_system_record(session, **kwargs):
 def delete_record(session, model, **kwargs):
     """ Deletes a record filtered by key(s) present in kwargs(contains model specific fields)."""
     keys = list(kwargs.keys())
-    instance = session.query(model).filter_by(**{k: kwargs[k] for k in keys}).first()
+    instance = session.scalars(db.select(model).filter_by(**{k: kwargs[k] for k in keys})).first()
     if instance:
         session.delete(instance)
         session.commit()

ros/processor/garbage_collector.py

@@ -24,22 +24,23 @@ def remove_outdated_data(self):
             time_value = datetime.now(timezone.utc) - timedelta(
                         days=DAYS_UNTIL_STALE)
             with app.app_context():
-                deleted_history = db.session.query(
-                    PerformanceProfileHistory).filter(
+                deleted_history = db.session.execute(
+                    db.delete(PerformanceProfileHistory).filter(
                         PerformanceProfileHistory.report_date < time_value
-                    ).delete()
+                    )
+                )
 
-                if deleted_history:
+                if deleted_history.rowcount > 0:
                     LOG.info(
                         f"{self.prefix} - Deleted {deleted_history} outdated history record(s) "
                         f"older than {DAYS_UNTIL_STALE} days"
                     )
 
-                deleted_profiles = db.session.query(
-                    PerformanceProfile
-                ).filter(PerformanceProfile.report_date < time_value).delete()
+                deleted_profiles = db.session.execute(
+                    db.delete(PerformanceProfile).filter(PerformanceProfile.report_date < time_value)
+                )
 
-                if deleted_profiles:
+                if deleted_profiles.rowcount > 0:
                     LOG.info(
                         f"{self.prefix} - Deleted {deleted_profiles} outdated performance profile(s) "
                         f"older than {DAYS_UNTIL_STALE} days"

ros/processor/insights_engine_consumer.py

@@ -94,10 +94,11 @@ def handle_msg(self, msg):
                 host['id'],
                 custom_prefix=self.prefix
             )
-            reports = msg["results"]["reports"]  \
-                if msg["results"]["reports"] \
-                and type(msg["results"]["reports"]) == list \
-                else []
+            if msg["results"]["reports"] \
+                    and type(msg["results"]["reports"]) == list:
+                reports = msg["results"]["reports"]
+            else:
+                reports = []
             ros_reports = [
                 report for report in reports
                 if 'ros_instance_evaluation' in report["rule_id"]
@@ -129,8 +130,8 @@ def process_report(self, host, platform_metadata, reports, system_metadata, perf
                     )
 
                 # get previous state of the system
-                system_previous_state = db.session.query(System.state) \
-                    .filter(System.inventory_id == host['id']).first()
+                system_previous_state = db.session.scalar(db.select(System.state)
+                                                          .filter(System.inventory_id == host['id']))
 
                 system_attrs = {
                     'tenant_id': account.id,
@@ -176,7 +177,7 @@ def process_report(self, host, platform_metadata, reports, system_metadata, perf
                     }
                     # max_io will be used to sort systems endpoint response instead of io
                     performance_utilization.update(
-                       {'max_io': max(performance_utilization['io'].values())}
+                        {'max_io': max(performance_utilization['io'].values())}
                     )
                 else:
                     LOG.debug(f"{self.prefix} - Setting default utilization for performance profile")
@@ -230,10 +231,10 @@ def process_report(self, host, platform_metadata, reports, system_metadata, perf
                 )
 
     def trigger_notification(
-        self, inventory_id, account, host, platform_metadata, system_previous_state, system_current_state
+            self, inventory_id, account, host, platform_metadata, system_previous_state, system_current_state
     ):
-        if system_previous_state[0] is not None:
-            if system_current_state not in (SYSTEM_STATES['OPTIMIZED'], system_previous_state[0]):
+        if system_previous_state is not None:
+            if system_current_state not in (SYSTEM_STATES['OPTIMIZED'], system_previous_state):
                 LOG.info(
                     f"{self.prefix} - Triggering a new suggestion event for the system: {inventory_id} belonging"
                     f" to account: {account.account} ({account.id}) and org_id: {account.org_id}"

ros/processor/inventory_events_consumer.py

@@ -97,10 +97,10 @@ def host_delete_event(self, msg):
                 f"{self.prefix} - Received a message for system with inventory_id {host_id}"
             )
 
-            db.session.execute(db.delete(System).filter(System.inventory_id == host_id))
+            rows_deleted = db.session.execute(db.delete(System).filter(System.inventory_id == host_id))
             db.session.commit()
-            system_exist = db.session.scalars(db.select(System).filter_by(inventory_id=host_id)).first()
-            if system_exist is None:
+
+            if rows_deleted.rowcount > 0:
                 processor_requests_success.labels(
                     reporter=self.reporter, org_id=msg['org_id']
                 ).inc()
@@ -138,7 +138,7 @@ def process_system_details(self, msg):
                 system = update_system_record(db.session, **system_fields)
                 if system is not None:
                     db.session.commit()
-                    account = db.session.query(RhAccount).filter_by(id=system.tenant_id).first()
+                    account = db.session.scalar(db.select(RhAccount).filter_by(id=system.tenant_id))
                     processor_requests_success.labels(
                         reporter=self.reporter, org_id=account.org_id
                     ).inc()

tests/helpers/db_helper.py

@@ -3,11 +3,11 @@
 
 
 def db_get_host(host_id):
-    return db.session.query(System).filter_by(inventory_id=host_id).first()
+    return db.session.scalar(db.select(System).filter_by(inventory_id=host_id))
 
 
 def db_get_record(model, **filters):
-    return db.session.query(model).filter_by(**filters).first()
+    return db.session.scalar(db.select(model).filter_by(**filters))
 
 
 def db_get_records(model, **filters):

tests/test_insights_engine_result_consumer.py

@@ -41,6 +41,7 @@ def _return_engine_msg_json(filename):
         msg_data = json.loads(f.read())
         f.close()
         return msg_data
+
     return _return_engine_msg_json
 
 
@@ -75,8 +76,8 @@ def test_process_report_idle(engine_result_message, engine_consumer, db_setup, p
         assert system_record.instance_type == _performance_record['instance_type']
         assert system_record.region == _performance_record['region']
         assert system_record.state == SYSTEM_STATES['INSTANCE_IDLE']
-        assert db.session.query(PerformanceProfile).filter_by(system_id=system_record.id).\
-               first().performance_record == performance_record
+        assert db.session.scalar(db.select(PerformanceProfile).filter_by(system_id=system_record.id))\
+            .performance_record == performance_record
 
 
 def test_process_report_under_pressure(engine_result_message, engine_consumer, db_setup, performance_record):
@@ -93,8 +94,8 @@ def test_process_report_under_pressure(engine_result_message, engine_consumer, d
         assert system_record.instance_type == _performance_record['instance_type']
         assert system_record.region == _performance_record['region']
         assert system_record.state == SYSTEM_STATES['INSTANCE_OPTIMIZED_UNDER_PRESSURE']
-        assert db.session.query(PerformanceProfile).filter_by(system_id=system_record.id).\
-               first().performance_record == performance_record
+        assert db.session.scalar(db.select(PerformanceProfile).filter_by(system_id=system_record.id))\
+            .performance_record == performance_record
 
 
 def test_process_report_no_pcp(engine_result_message, engine_consumer, db_setup, performance_record):
@@ -106,8 +107,8 @@ def test_process_report_no_pcp(engine_result_message, engine_consumer, db_setup,
     _performance_record = copy.copy(performance_record)
     engine_consumer.process_report(host, platform_metadata, ros_reports, system_metadata, performance_record)
     system_record = db_get_host(host['id'])
-    performance_utilization = db.session.query(PerformanceProfile).\
-        filter_by(system_id=system_record.id).first().performance_utilization
+    performance_utilization = db.session.scalar(db.select(PerformanceProfile)
+                                                .filter_by(system_id=system_record.id)).performance_utilization
     sample_performance_util_no_pcp = {'cpu': -1, 'memory': -1, 'max_io': -1.0, 'io': {}}
     assert str(system_record.inventory_id) == host['id']
     with app.app_context():
@@ -131,8 +132,8 @@ def test_process_report_undersized(engine_result_message, engine_consumer, db_se
         assert system_record.instance_type == _performance_record['instance_type']
         assert system_record.region == _performance_record['region']
         assert system_record.state == SYSTEM_STATES['INSTANCE_UNDERSIZED']
-        assert db.session.query(PerformanceProfile).filter_by(system_id=system_record.id).\
-               first().performance_record == performance_record
+        assert db.session.scalar(db.select(PerformanceProfile).filter_by(system_id=system_record.id))\
+            .performance_record == performance_record
 
 
 def test_process_report_optimized(engine_result_message, engine_consumer, db_setup, performance_record):
@@ -151,8 +152,8 @@ def test_process_report_optimized(engine_result_message, engine_consumer, db_set
         assert system_record.instance_type == _performance_record['instance_type']
         assert system_record.region == _performance_record['region']
         assert system_record.state == SYSTEM_STATES['OPTIMIZED']
-        assert db.session.query(PerformanceProfile).filter_by(system_id=system_record.id).\
-               first().performance_record == performance_record
+        assert db.session.scalar(db.select(PerformanceProfile).filter_by(system_id=system_record.id))\
+            .performance_record == performance_record
 
 
 def test_system_properties(engine_result_message, engine_consumer, db_setup, performance_record):
@@ -214,8 +215,8 @@ def test_process_report_psi_enabled(engine_result_message, engine_consumer, db_s
     platform_metadata = engine_result_message["input"]["platform_metadata"]
     engine_consumer.process_report(host, platform_metadata, ros_reports, system_metadata, performance_record)
     system_record = db_get_host(host['id'])
-    psi_enabled = db.session.query(PerformanceProfile).\
-        filter_by(system_id=system_record.id).first().psi_enabled
+    psi_enabled = db.session.scalar(db.select(PerformanceProfile).filter_by(system_id=system_record.id))\
+                    .psi_enabled
     assert psi_enabled is True
 
 
@@ -229,7 +230,7 @@ def test_notification(engine_result_message, engine_consumer, db_setup, performa
     engine_consumer.process_report(host, platform_metadata, ros_reports, system_metadata, performance_record)
     system_record = db_get_host(host['id'])
     response = notification_payload(
-                    host, system_previous_state, system_record.state)
+        host, system_previous_state, system_record.state)
 
     assert response["account_id"] == host["account"]
     assert response["context"]["display_name"] == "ip-172-31-28-69.ec2.internal"