Upgrade to Python 3.8

.circleci/config.yml

@@ -7,7 +7,7 @@ version: 2.1
 jobs:
   tests:
     docker:
-      - image: 'cimg/python:3.6'
+      - image: 'cimg/python:3.8'
     steps:
       - checkout
       - run: pip install --user -r requirements/testing.txt
@@ -16,7 +16,7 @@ jobs:
 
   checks:
     docker:
-      - image: 'cimg/python:3.6'
+      - image: 'cimg/python:3.8'
     steps:
       - checkout
       - run: pip install --user -r requirements/testing.txt

.envs/local/django

@@ -24,9 +24,16 @@ DATABASE_URL=psql://localuser:localpass@postgres:5432/ethicaladserver
 #SLACK_TOKEN=
 
 
-#
+# Metabase
 # ------------------------------------------------------------------------------
 # This is the address in docker. Use http://localhost:3000 outside docker.
 METABASE_SITE_URL=http://metabase:3000
 # Ensure this is set in http://localhost:3000/admin/settings/embedding_in_other_applications
 METABASE_SECRET_KEY=0000000000000000000000000000000000000000000000000000000000000000
+
+
+# Celery
+# ------------------------------------------------------------------------------
+# This is a workaround for some celery issues that are likely fixed in future versions.
+# https://github.com/celery/celery/issues/5761
+COLUMNS=80

adserver/admin.py

@@ -46,12 +46,14 @@ class RemoveDeleteMixin:
     """Removes the ability to delete this model from the admin."""
 
     def get_actions(self, request):
-        actions = super(RemoveDeleteMixin, self).get_actions(request)
+        actions = super().get_actions(request)
         if "delete_selected" in actions:
             del actions["delete_selected"]  # pragma: no cover
         return actions
 
-    def has_delete_permission(self, request, obj=None):
+    def has_delete_permission(
+        self, request, obj=None
+    ):  # pylint: disable=unused-argument
         return False
 
 
@@ -96,11 +98,9 @@ def report(self, instance):
         if not instance.pk:
             return ""  # pragma: no cover
 
-        return mark_safe(
-            '<a href="{url}">{name}</a>'.format(
-                name=escape(instance.name) + " Report", url=instance.get_absolute_url()
-            )
-        )
+        name = escape(instance.name)
+        url = instance.get_absolute_url()
+        return mark_safe(f'<a href="{url}">{name}</a> Report')
 
 
 class AdvertiserAdmin(RemoveDeleteMixin, SimpleHistoryAdmin):
@@ -536,7 +536,7 @@ def action_create_draft_invoice(self, request, queryset):
     )
 
     def get_queryset(self, request):
-        queryset = super(FlightAdmin, self).get_queryset(request)
+        queryset = super().get_queryset(request)
         queryset = queryset.annotate(
             num_ads=models.Count("advertisements", distinct=True)
         )
@@ -636,7 +636,7 @@ def ecpm(self, obj):
         return "${:.2f}".format(calculate_ecpm(obj.total_value(), views))
 
     def get_queryset(self, request):
-        queryset = super(CampaignAdmin, self).get_queryset(request)
+        queryset = super().get_queryset(request)
         queryset = queryset.annotate(
             campaign_total_value=models.Sum(
                 (

adserver/api/views.py

@@ -356,7 +356,7 @@ def get_queryset(self):
         return self.request.user.advertisers.all()
 
     @action(detail=True, methods=["get"])
-    def report(self, request, slug=None):
+    def report(self, request, slug=None):  # pylint: disable=unused-argument
         """Return a report of ad performance for this advertiser."""
         # This will raise a 404 if the user doesn't have access to the advertiser
         advertiser = self.get_object()
@@ -465,7 +465,7 @@ def get_queryset(self):
         return self.request.user.publishers.all()
 
     @action(detail=True, methods=["get"])
-    def report(self, request, slug=None):
+    def report(self, request, slug=None):  # pylint: disable=unused-argument
         """Return a report of ad performance for this publisher."""
         # This will raise a 404 if the user doesn't have access to the publisher
         publisher = self.get_object()

adserver/management/commands/archive_offers.py

@@ -94,7 +94,7 @@ def handle_archive_day(self, day):
                 ORDER BY date
             ) TO STDOUT WITH CSV HEADER"""
         with connection[settings.REPLICA_SLUG].cursor() as cursor:
-            with open(output_file, "w") as fd:
+            with open(output_file, "wb") as fd:
                 # https://www.psycopg.org/docs/cursor.html#cursor.copy_expert
                 cursor.copy_expert(query, fd)
 

adserver/management/commands/rtdimport.py

@@ -296,6 +296,27 @@ def import_advertisements(self, advertisements_data):
             "small",
         ]
 
+        with open(
+            os.path.join(self.BASE_DIR, "adtype-templates/readthedocs-sidebar.html"),
+            "r",
+            encoding="utf-8",
+        ) as fd:
+            rtd_sidebar_template = fd.read()
+        with open(
+            os.path.join(self.BASE_DIR, "adtype-templates/readthedocs-footer.html"),
+            "r",
+            encoding="utf-8",
+        ) as fd:
+            rtd_footer_template = fd.read()
+        with open(
+            os.path.join(
+                self.BASE_DIR, "adtype-templates/readthedocs-fixedfooter.html"
+            ),
+            "r",
+            encoding="utf-8",
+        ) as fd:
+            rtd_fixedfooter_template = fd.read()
+
         ad_type_mapping = {
             "doc": AdType.objects.create(
                 name="RTD Sidebar",
@@ -304,12 +325,7 @@ def import_advertisements(self, advertisements_data):
                 has_text=True,
                 max_text_length=150,  # Many ads exceed the "allowed" 80
                 allowed_html_tags=" ".join(allowed_tags),
-                template=open(
-                    os.path.join(
-                        self.BASE_DIR, "adtype-templates/readthedocs-sidebar.html"
-                    ),
-                    "r",
-                ).read(),
+                template=rtd_sidebar_template,
             ),
             "site-footer": AdType.objects.create(
                 name="RTD Footer",
@@ -320,12 +336,7 @@ def import_advertisements(self, advertisements_data):
                 has_text=True,
                 max_text_length=300,
                 allowed_html_tags=" ".join(allowed_tags),
-                template=open(
-                    os.path.join(
-                        self.BASE_DIR, "adtype-templates/readthedocs-footer.html"
-                    ),
-                    "r",
-                ).read(),
+                template=rtd_footer_template,
             ),
             "fixed-footer": AdType.objects.create(
                 name="RTD Fixed Footer",
@@ -334,12 +345,7 @@ def import_advertisements(self, advertisements_data):
                 has_text=True,
                 max_text_length=100,
                 allowed_html_tags=" ".join(allowed_tags),
-                template=open(
-                    os.path.join(
-                        self.BASE_DIR, "adtype-templates/readthedocs-fixedfooter.html"
-                    ),
-                    "r",
-                ).read(),
+                template=rtd_fixedfooter_template,
             ),
             # There are two "error" ads but they are old and problematic
             # The images are SVGs (can't be stored in an ImageField)
@@ -529,5 +535,5 @@ def calculate_flight_totals(self):
             flight.save()
 
         self.stdout.write(
-            self.style.SUCCESS(f"- Calculated totals across imported flights")
+            self.style.SUCCESS("- Calculated totals across imported flights")
         )

adserver/mixins.py

@@ -236,7 +236,8 @@ class AllReportMixin:
 
     """A mixin that handles the primary "view" logic for staff reports."""
 
-    def get_context_data(self, **kwargs):  # pylint: disable=missing-docstring
+    def get_context_data(self, **kwargs):
+        """Set the base data needed for all reports."""
         context = super().get_context_data(**kwargs)
 
         sort = self.request.GET.get("sort", "")

adserver/templatetags/metabase.py

@@ -28,8 +28,7 @@ def metabase_question_embed(question_id, **kwargs):
     # These parameters must be JSON serializable to be signed
     # Notably, dates aren't serializable by default
     params = {}
-    for k in kwargs:
-        val = kwargs[k]
+    for (k, val) in kwargs.items():
         if isinstance(val, (date, datetime)):
             params[k] = str(val)
         else:

adserver/utils.py

@@ -285,8 +285,8 @@ def build_blocked_ip_set():
 
     filepath = os.path.join(settings.GEOIP_PATH, "torbulkexitlist.txt")
     if os.path.exists(filepath):
-        with open(filepath) as fd:
-            for line in fd.readlines():
+        with open(filepath, "r", encoding="utf-8") as fd:
+            for line in fd:
                 line = line.strip()
                 if line:
                     blocked_ips.add(line)
@@ -344,7 +344,9 @@ def generate_publisher_payout_data(
 
     TODO: Break this function up and move it out of utils.
     """
-    from .reports import PublisherReport  # pylint: disable=cyclic-import
+    # pylint: disable=cyclic-import
+    # pylint: disable=import-outside-toplevel
+    from .reports import PublisherReport
 
     today = timezone.now()
     end_date = today.replace(day=1) - timedelta(days=1)

adserver/views.py

@@ -217,7 +217,7 @@ def get_context_data(self, **kwargs):
         )
         return context
 
-    def get_object(self, queryset=None):  # pylint: disable=unused-argument
+    def get_object(self, queryset=None):
         self.advertiser = get_object_or_404(
             Advertiser, slug=self.kwargs["advertiser_slug"]
         )
@@ -231,7 +231,7 @@ class FlightListView(AdvertiserAccessMixin, UserPassesTestMixin, ListView):
     model = Flight
     template_name = "adserver/advertiser/flight-list.html"
 
-    def get_context_data(self, **kwargs):  # pylint: disable=arguments-differ
+    def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
 
         context.update({"advertiser": self.advertiser, "flights": self.get_queryset()})
@@ -262,7 +262,7 @@ def get_context_data(self, **kwargs):
         )
         return context
 
-    def get_object(self, queryset=None):  # pylint: disable=unused-argument
+    def get_object(self, queryset=None):
         self.advertiser = get_object_or_404(
             Advertiser, slug=self.kwargs["advertiser_slug"]
         )
@@ -338,7 +338,7 @@ def get_context_data(self, **kwargs):
         context.update({"advertiser": self.advertiser})
         return context
 
-    def get_object(self, queryset=None):  # pylint: disable=unused-argument
+    def get_object(self, queryset=None):
         self.advertiser = get_object_or_404(
             Advertiser, slug=self.kwargs["advertiser_slug"]
         )
@@ -370,7 +370,7 @@ def get_context_data(self, **kwargs):
         context.update({"advertiser": self.advertiser})
         return context
 
-    def get_object(self, queryset=None):  # pylint: disable=unused-argument
+    def get_object(self, queryset=None):
         self.advertiser = get_object_or_404(
             Advertiser, slug=self.kwargs["advertiser_slug"]
         )
@@ -411,7 +411,7 @@ def get_form_kwargs(self):
         kwargs["flight"] = ad.flight
         return kwargs
 
-    def get_object(self, queryset=None):  # pylint: disable=unused-argument
+    def get_object(self, queryset=None):
         self.advertiser = get_object_or_404(
             Advertiser, slug=self.kwargs["advertiser_slug"]
         )
@@ -1225,7 +1225,7 @@ class AdvertiserAuthorizedUsersView(
     model = get_user_model()
     template_name = "adserver/advertiser/users.html"
 
-    def get_context_data(self, **kwargs):  # pylint: disable=arguments-differ
+    def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
         context.update({"advertiser": self.advertiser})
         return context
@@ -1696,7 +1696,7 @@ def get_context_data(self, **kwargs):
         context.update({"publisher": self.object})
         return context
 
-    def get_object(self, queryset=None):  # pylint: disable=unused-argument
+    def get_object(self, queryset=None):
         return get_object_or_404(Publisher, slug=self.kwargs["publisher_slug"])
 
     def get_success_url(self):
@@ -1802,19 +1802,20 @@ class PublisherPayoutListView(PublisherAccessMixin, UserPassesTestMixin, ListVie
     model = PublisherPayout
     template_name = "adserver/publisher/payout-list.html"
 
-    def get_context_data(self, **kwargs):  # pylint: disable=arguments-differ
+    def get_context_data(self, **kwargs):
         """Get the past payouts, along with the current balance and future balance."""
         context = super().get_context_data(**kwargs)
 
         payouts = self.get_queryset()
         data = generate_publisher_payout_data(self.publisher)
+        total_balance = float(self.publisher.total_payout_sum())
 
-        total_balance = (
-            float(self.publisher.total_payout_sum())
-            + data["current_report"]["total"]["revenue_share"]
-        )
+        # For some reason, pylint is confused by the deep nesting
+        # pylint: disable=unsubscriptable-object
+        if data["current_report"]:
+            total_balance += data["current_report"]["total"]["revenue_share"]
 
-        if data.get("due_report"):
+        if data["due_report"]:
             total_balance += data["due_report"]["total"]["revenue_share"]
 
         context.update(data)
@@ -2108,7 +2109,7 @@ def get_context_data(self, **kwargs):
         )
         return context
 
-    def get_object(self, queryset=None):  # pylint: disable=unused-argument
+    def get_object(self, queryset=None):
         self.publisher = get_object_or_404(
             Publisher, slug=self.kwargs["publisher_slug"]
         )
@@ -2203,7 +2204,7 @@ def post(self, request, *args, **kwargs):
         messages.info(request, _("API token revoked"))
         return result
 
-    def get_object(self, queryset=None):  # pylint: disable=unused-argument
+    def get_object(self, queryset=None):
         token = Token.objects.filter(user=self.request.user).first()
         if not token:
             raise Http404

config/celery_app.py

@@ -13,4 +13,4 @@
 
 @app.task(bind=True)
 def debug_task(self):
-    print("Request: {0!r}".format(self.request))
+    print(f"Request: {self.request!r}")

config/settings/base.py

@@ -116,10 +116,11 @@
 # Database
 # https://docs.djangoproject.com/en/2.2/ref/settings/#databases
 # --------------------------------------------------------------------------
+DB_PATH_SQLITE = os.path.join(BASE_DIR, "db.sqlite3")
 DATABASES = {
     "default": env.db(
         "DATABASE_URL",
-        default="sqlite:///{}".format(os.path.join(BASE_DIR, "db.sqlite3")),
+        default=f"sqlite:///{DB_PATH_SQLITE}",
     )
 }
 DATABASES["default"]["ATOMIC_REQUESTS"] = True
@@ -446,5 +447,5 @@
 ADSERVER_SUPPORT_TO_EMAIL = env("ADSERVER_SUPPORT_TO_EMAIL", default=None)
 ADSERVER_SUPPORT_FORM_ACTION = env("ADSERVER_SUPPORT_FORM_ACTION", default=None)
 
-with open(os.path.join(BASE_DIR, "package.json")) as fd:
+with open(os.path.join(BASE_DIR, "package.json"), encoding="utf-8") as fd:
     ADSERVER_VERSION = json.load(fd)["version"]

config/urls.py

@@ -44,7 +44,7 @@
     # In development, the Django admin is available at /admin
     # In production, a custom URL path can be specified
     # If no ADSERVER_ADMIN_URL is specified, the Django admin is disabled
-    urlpatterns += [path(r"{}/".format(settings.ADSERVER_ADMIN_URL), admin.site.urls)]
+    urlpatterns += [path(f"{settings.ADSERVER_ADMIN_URL}/", admin.site.urls)]
 
 if "djstripe" in settings.INSTALLED_APPS:
     urlpatterns += [

docker-compose/local/django/Dockerfile

@@ -1,4 +1,4 @@
-FROM python:3.6-alpine
+FROM python:3.8-alpine
 
 ENV PYTHONUNBUFFERED 1
 

docker-compose/production/django/Dockerfile

@@ -3,7 +3,7 @@
 # This Dockerfile is not currently used or maintained
 # but may be useful in setting up your own adserver instance.
 
-FROM python:3.6-alpine
+FROM python:3.8-alpine
 MAINTAINER Read the Docs, Inc. <[email protected]>
 
 ENV PYTHONUNBUFFERED 1