Problem saving image from URL to imageField

I am having trouble figuring out my problem regarding saving an image from an URL.

The problem is in the _fetch_image_from_url method.
I get the error:

Exception Value:	'NoneType' object has no attribute 'save'
Exception Location:	/home/andrej/github/curated/hosted/controller.py, line 64, in _fetch_image_from_url

I am not sure why this happens and cant get my head around it. Therefore I am reaching out.

For context here my code;

model:

class App(models.Model):
    app_logo = models.ImageField(
        upload_to='logos',
        null=True,
        blank=True
    )
    created_at = models.DateTimeField(auto_now_add=True)
    updated_at = models.DateTimeField(auto_now=True)
    repo = models.CharField(blank=True, null=True, editable=False, max_length=300)
    owner = models.CharField(blank=True, null=True, editable=False, max_length=300)
    github_url = models.URLField(default="", blank=False, max_length=300, unique=True)
[...]

The view is calling my “Controller”.

class AppCreateView(LoginRequiredMixin, CreateView):
    form_class = AppModelForm
    template_name = 'hosted/app_create_view.html'

    def form_valid(self, form):
        controller = CreateAppController(form.cleaned_data['github_url'])
        app = controller.create()
        return HttpResponseRedirect("/self-hosted")

The controller:

from urllib.parse import urlsplit

import requests
from django.core.files.base import ContentFile
from github import Github
from github.GithubException import RateLimitExceededException

from curated.settings.base import GITHUB_API_TOKEN
from hosted.models import App
from django.core.files import File



class CreateAppController:
    def __init__(self, github_url):
        self.owner = None
        self.repo = None
        self.github_url = github_url
        self.github_stars = None
        self.github_description = None
        self.github_homepage_url = None
        self.github_organization_avatar_url = None
        self.app = None
        self.app_logo = None
        self.github_rate_limited = False

    def create(self):
        self._split_repo_url()
        self._fetch_github_repo_data()
        self._fetch_image_from_url()
        self.create_app_instance()
        return self.app

    def _split_repo_url(self):
        self.owner = urlsplit(str(self.github_url)).path.split('/')[1]
        self.repo = urlsplit(str(self.github_url)).path.split('/')[2]


    def _fetch_github_repo_data(self):
        g = Github(GITHUB_API_TOKEN)
        print(g.rate_limiting)

        try:
            app_data = g.get_repo(self.owner + "/" + self.repo)
            self.github_stars = app_data.stargazers_count
            self.github_description = app_data.description
            self.github_homepage_url = app_data.homepage

            if hasattr(app_data.organization, 'avatar_url'):
                self.github_organization_avatar_url = app_data.organization.avatar_url
            if hasattr(app_data.owner, 'avatar_url'):
                self.github_organization_avatar_url = app_data.owner.avatar_url

        except RateLimitExceededException as e:
            self.github_rate_limited = True
            print(f"Rate limit exceeded. Exception: {e}")

    def _fetch_image_from_url(self):
        response = requests.get(self.github_organization_avatar_url)
        if response.status_code == 200:
            content = ContentFile(response.content)
            file_name = f'{self.owner}_{self.repo}.jpg'
            self.app_logo.save(
                file_name,
                File(content),
                save=False
            )

    def create_app_instance(self):
        self.app = App(
            owner=self.owner,
            repo=self.repo,
            github_url=self.github_url,
            github_stars=self.github_stars,
            github_description=self.github_description,
            github_homepage_url=self.github_homepage_url,
            github_organization_avatar_url=self.github_organization_avatar_url,
            github_rate_limited=self.github_rate_limited,
            app_logo=self.app_logo
        )

        self.app.save()


PS: This code might not be the django way. The controller idea is from a friend who is really experienced in Python but not with Django. But I liked to keep the logic out of the model and the view.

I don’t see any location where this is being created/set. self is an instance of CreateAppController, not App.

(And yes, I’d do myself a favor and refactor this promptly. I wouldn’t want to have to come back to this in six months as it is…)

Thanks, that was the right hint. I actually assumed it was an App instance.

Could you elaborate on this?
I had all these methods first on the model and just created a custom save method in order to save an instance of that App model.

However, I have a usecase where I only want to update certain values from the github API and for that I need a new save method again, the update_github_stars method for example.
With the controller, I simply could create a Controller called Update Github Stars and can implement the logic in there.
Same thing applies when I want to add GitLab projects which will have a different API and therefore different logic.

I don’t know how I would approach this and my friend pitched me this which I just adapted.
What would be the “django” or lets say easier way to accomplish something like this?
This is a one-person project (my hobby learning thing) so I do not need a complex design for team structures, however, I like to have an easier time developing and being clear about where things need to be placed.

Sure! I’ll do my best here. Primarily by backtracking on my earlier response and admitting that I misread parts of the code. (oops) So, I’ll start by saying that it’s not as bad as my first reaction would imply. I still would do it differently though.

I guess what I find most “inelegent” about this approach is the creation of an object that’s not needed, to hold intermediate results, only to copy those results to the actual object when it’s being created and saved.

I would consider integrating this functionality within a combination of the model and the manager.

In this case, I’d start by looking just adding these methods to the model, except for the __init__ method - most of that can probably be disposed of, and the create method that drives this process, which belongs somewhere else.

If I were doing this, I would override the create method on the manager. This allows me to use the api that I use with every other object, and provides a facility for me to pass parameters in to the creation process when different actions need to be taken. (I don’t know if there’s any different circumstance in which one of these App objects need to be created, making this a decision that should be made in the context of the overall project.)

Do you happen to know a repo where I could see this approach?

What aspects of this are you looking to see illustrated? The creation of a custom manager? Or something else?

The mat model approach and the custom managers.
Some kind of open-source project where I can have a look at the structure and adjust accordingly.

I’m not specifically aware of any such repo, no.

Creating a manager is well documented at Managers | Django documentation | Django.

You could be creating a new create method in that class, again depending upon your potential need for the standard create method.

So it might look like this:

# Structure derrived from django.db.models.query.QuerySet
def create(self,  **kwargs):
        obj = self.model(**kwargs)
        obj._split_repo_url()
        obj._fetch_github_repo_data()
        obj._fetch_image_from_url()
        obj.save()

Your _split_repo_url, _fetch_github_repo_data, and _fetch_image_from_url methods do not look like they need to be changed at all when placed in the model.