Browse Source

fix(py3): Fix more unsafe uses of dict views. (#20874)

This fixes a class of errors where we modify a dict that we're iterating over. This is fine in py2
since we're working on a copy of the keys/etc, but in py3 since it's a view it will error. We also
need to do this for implicit uses of keys as well, ie:

```
for key in my_dict:
    my_dict[key] = 'abc'
```

Will follow up with that separately.
Dan Fuller 4 years ago
parent
commit
4fb7d49486

+ 6 - 5
api-docs/generator.py

@@ -3,17 +3,18 @@ from __future__ import absolute_import
 
 import click
 import docker
-import json
 import os
 import six
 import zlib
-from datetime import datetime
 from contextlib import contextmanager
+from datetime import datetime
+from subprocess import Popen
+
+from sentry.conf.server import SENTRY_DEVSERVICES
 from sentry.runner.commands.devservices import get_docker_client, get_or_create
+from sentry.utils import json
 from sentry.utils.apidocs import MockUtils, iter_scenarios, iter_endpoints, get_sections
 from sentry.utils.integrationdocs import sync_docs
-from sentry.conf.server import SENTRY_DEVSERVICES
-from subprocess import Popen
 
 HERE = os.path.abspath(os.path.dirname(__file__))
 OUTPUT_PATH = "/usr/src/output"
@@ -84,7 +85,7 @@ def apidoc_containers():
 
     # Massage our list into some shared settings instead of repeating
     # it for each definition.
-    for name, options in containers.items():
+    for name, options in list(containers.items()):
         options["network"] = namespace
         options["detach"] = True
         options["name"] = namespace + "_" + name

+ 1 - 1
src/sentry/discover/utils.py

@@ -108,7 +108,7 @@ def transform_aliases_and_query(**kwargs):
         elif isinstance(aggregation[1], (set, tuple, list)):
             aggregation[1] = [get_snuba_column_name(col) for col in aggregation[1]]
 
-    for col in filter_keys.keys():
+    for col in list(filter_keys.keys()):
         name = get_snuba_column_name(col)
         filter_keys[name] = filter_keys.pop(col)
 

+ 2 - 2
src/sentry/rules/conditions/event_attribute.py

@@ -54,7 +54,7 @@ ATTR_CHOICES = [
 
 class EventAttributeForm(forms.Form):
     attribute = forms.ChoiceField((a, a) for a in ATTR_CHOICES)
-    match = forms.ChoiceField(MATCH_CHOICES.items())
+    match = forms.ChoiceField(list(MATCH_CHOICES.items()))
     value = forms.CharField(widget=forms.TextInput(), required=False)
 
 
@@ -84,7 +84,7 @@ class EventAttributeCondition(EventCondition):
             "placeholder": "i.e. exception.type",
             "choices": [[a, a] for a in ATTR_CHOICES],
         },
-        "match": {"type": "choice", "choices": MATCH_CHOICES.items()},
+        "match": {"type": "choice", "choices": list(MATCH_CHOICES.items())},
         "value": {"type": "string", "placeholder": "value"},
     }
 

+ 6 - 8
src/sentry/runner/commands/devservices.py

@@ -121,10 +121,10 @@ def up(services, project, exclude, fast):
     for service in exclude:
         if service not in containers:
             click.secho(
-                "Service `{}` is not known or not enabled.\n".format(service), err=True, fg="red",
+                "Service `{}` is not known or not enabled.\n".format(service), err=True, fg="red"
             )
             click.secho(
-                "Services that are available:\n" + "\n".join(containers.keys()) + "\n", err=True,
+                "Services that are available:\n" + "\n".join(containers.keys()) + "\n", err=True
             )
             raise click.Abort()
 
@@ -138,8 +138,7 @@ def up(services, project, exclude, fast):
                     fg="red",
                 )
                 click.secho(
-                    "Services that are available:\n" + "\n".join(containers.keys()) + "\n",
-                    err=True,
+                    "Services that are available:\n" + "\n".join(containers.keys()) + "\n", err=True
                 )
                 raise click.Abort()
             selected_containers[service] = containers[service]
@@ -208,7 +207,7 @@ def _start_service(client, name, containers, project, fast=False, always_start=F
         options["environment"].pop("DEFAULT_BROKERS", None)
         options["command"] = ["devserver", "--no-workers"]
 
-    for key, value in options["environment"].items():
+    for key, value in list(options["environment"].items()):
         options["environment"][key] = value.format(containers=containers)
 
     pull = options.pop("pull", False)
@@ -225,7 +224,7 @@ def _start_service(client, name, containers, project, fast=False, always_start=F
                 click.secho("> Pulling image '%s'" % options["image"], err=True, fg="green")
                 client.images.pull(options["image"])
 
-    for mount in options.get("volumes", {}).keys():
+    for mount in list(options.get("volumes", {}).keys()):
         if "/" not in mount:
             get_or_create(client, "volume", project + "_" + mount)
             options["volumes"][project + "_" + mount] = options["volumes"].pop(mount)
@@ -350,8 +349,7 @@ def rm(project, services):
                     fg="red",
                 )
                 click.secho(
-                    "Services that are available:\n" + "\n".join(containers.keys()) + "\n",
-                    err=True,
+                    "Services that are available:\n" + "\n".join(containers.keys()) + "\n", err=True
                 )
                 raise click.Abort()
             selected_containers[service] = containers[service]

+ 1 - 1
src/sentry/tsdb/inmemory.py

@@ -229,7 +229,7 @@ class InMemoryTSDB(BaseTSDB):
             for timestamp in series:
                 result.update(source[self.normalize_ts_to_rollup(timestamp, rollup)])
 
-        for key, counter in results.items():
+        for key, counter in list(results.items()):
             results[key] = counter.most_common(limit)
 
         return results

+ 1 - 1
src/sentry/tsdb/redis.py

@@ -791,7 +791,7 @@ class RedisTSDB(BaseTSDB):
         # as positional arguments to the Redis script and later associating the
         # results (which are returned in the same order that the arguments were
         # provided) with the original input values to compose the result.
-        for key, members in items.items():
+        for key, members in list(items.items()):
             items[key] = list(members)
 
         commands = {}

+ 7 - 9
src/sentry/tsdb/snuba.py

@@ -326,7 +326,7 @@ class SnubaTSDB(BaseTSDB):
         if len(groups) > 0:
             group, subgroups = groups[0], groups[1:]
             if isinstance(result, dict):
-                for rk in result.keys():
+                for rk in list(result.keys()):
                     if group == "time":  # Skip over time group
                         self.trim(result[rk], subgroups, keys)
                     elif rk in keys:
@@ -455,15 +455,13 @@ class SnubaTSDB(BaseTSDB):
         #    {group:{timestamp:[top1, ...]}}
         # into
         #    {group: [(timestamp, {top1: score, ...}), ...]}
-        for k in result:
-            result[k] = sorted(
-                [
-                    (timestamp, {v: float(i + 1) for i, v in enumerate(reversed(topk or []))})
-                    for (timestamp, topk) in result[k].items()
-                ]
+        return {
+            k: sorted(
+                (timestamp, {v: float(i + 1) for i, v in enumerate(reversed(topk or []))})
+                for (timestamp, topk) in result[k].items()
             )
-
-        return result
+            for k in result.keys()
+        }
 
     def get_frequency_series(self, model, items, start, end=None, rollup=None, environment_id=None):
         result = self.get_data(

+ 1 - 1
src/sentry/utils/apidocs.py

@@ -578,7 +578,7 @@ class Runner(object):
                 headers["Content-Type"] = "application/json"
             elif format == "multipart":
                 files = {}
-                for key, value in data.items():
+                for key, value in list(data.items()):
                     if hasattr(value, "read") or isinstance(value, tuple):
                         files[key] = value
                         del data[key]