Upgrade Django because #3297 #3305

kakulukia · 2017-11-23T14:45:22Z

Fixed incompatibility with geos 3.6.3 while upgrading Django to its latest version.
Aditionally upgradead all other libs to the latest version except elasticsearch. Refactored render_to_request() to render()

kakulukia · 2017-11-23T15:44:17Z

not sure about hos errers - i cant figure out what this has to do with my changes :(

humitos

I think the overall is OK. Just some comments:

I'm not sure about the Pipfile since I never used it, but seems to be something good to have
I think at RTD are kind of "conservative" in some way, regarding upgrading all the packages at once, so I'm not sure that something that they want (Upgrade all packages using pur tool #2916)
There are some test that do not pass with your changes (maybe it's because of the upgrades), can you fix them?

Finally, can you take a look at my comments?

humitos · 2017-11-24T00:27:42Z

readthedocs/settings/base.py

    TASTYPIE_FULL_DEBUG = True

    # Domains and URLs
    PRODUCTION_DOMAIN = 'readthedocs.org'
    PUBLIC_DOMAIN = None
-    USE_SUBDOMAIN = False
+    USE_SUBDOMAIN = True


Why you changed this default?

@kakulukia Any feedback on this?

this shouldnt have made it into the PR - its already reverted and github anlready maks this as outdated for me

humitos · 2017-11-24T00:28:04Z

readthedocs/settings/base.py

@@ -274,7 +278,7 @@ def INSTALLED_APPS(self):  # noqa

    # RTD Settings
    REPO_LOCK_SECONDS = 30
-    ALLOW_PRIVATE_REPOS = False
+    ALLOW_PRIVATE_REPOS = True


Why you changed this default?

humitos · 2017-11-24T00:28:38Z

readthedocs/settings/base.py

+    TEMPLATES = [
+        {
+            'BACKEND': 'django.template.backends.django.DjangoTemplates',
+            'DIRS': [os.path.join(SITE_ROOT, 'readthedocs', 'templates')],


Use TEMPLATE_ROOT here.

kakulukia · 2017-11-24T09:15:04Z

Hi @humitos i didnt mean to change the config defaults .. had to change them for my installation, but reverted that in the commits.
TEMPLATE_ROOT is not supported anymore - see: https://docs.djangoproject.com/en/1.8/ref/templates/upgrading/
And if im remembering correct, the project was already using Django 1.9 before my changes, so no harm done even for the old version.

kakulukia · 2017-11-24T09:31:06Z

I also pushed a version of pip.txt with minimal changes .. lets see how Travis likes that .. btw, the errors happened in the pagination package, but now there is a GDAL error .. i dont know how to influence Travis in being able to install it .. looks weird!

@humitos i dont see how my changes would influence the installation of gdal?
Any ideas?

humitos · 2017-11-24T18:08:10Z

TEMPLATE_ROOT is not supported anymore - see: docs.djangoproject.com/en/1.8/ref/templates/upgrading

I think it's something used internally by RTD, it's used in a couple of places. That's why I said that it should be used in the DIRS key for the templates.

@humitos i dont see how my changes would influence the installation of gdal? any ideas?

Not really :(

kakulukia · 2017-11-24T19:27:37Z

I misunderstood that part .. its replaced now

kakulukia · 2017-11-24T19:29:08Z

I still dont know why it wouldnt build - somebody with more knowledge about the travis setup should have a look

kakulukia · 2017-11-24T20:03:30Z

The new Django version might be responsible for that GDAL error .. but is RTD actually using GDAL?

kakulukia · 2017-11-25T06:53:23Z

That didnt work out that well ..

tony · 2017-12-17T14:11:28Z

Let's keep #3297 open since that's not fixed until we get something merged

tony · 2017-12-17T14:17:32Z

#3297 is blocking me on #3341. So let's get something squared away.

Re: Pipfile: I was thinking about a Pipfile, too. But that should happen in a separate PR. It's best to keep this PR conservative as possible. You'll notice a lot of future tweaking opportunities, so it's a good chance to write them down and tackle them in issues.

At this point it needs a rebase again master.

Are you open to sharing your repo and teaming up on it @kakulukia ?

kakulukia · 2017-12-17T18:18:48Z

@tony you are welcome to join!

kakulukia · 2017-12-18T13:24:59Z

Well @tony as i stated before "I still dont know why it wouldnt build - somebody with more knowledge about the travis setup should have a look"

tony · 2017-12-18T13:54:17Z

For some reason my change at 7cc235a isn't showing up on Travis.

tony · 2017-12-18T14:15:02Z

I'm concerned about tweaks introduced outside the scope of fixing #3297. Most of them seem ironed out though. The official solution appears to be Django 1.11 (https://code.djangoproject.com/ticket/28441).

For some backstory: Here's the change that was done to fix #3297 itself in 1.11: django/django@a49764d.

tony · 2017-12-18T14:17:17Z

@kakulukia I created two branches in your repo: django-1.11 (where I'm going to be doing squashing / deleting / rebasing) and django 1.11-backup.

tony · 2017-12-18T14:19:34Z

@kakulukia Close this PR, make a new PR with this: master...kakulukia:django-1.11

That way you're operating off a branch name that doesn't clobber with the origin (main RTD's) master branch.

kakulukia · 2017-12-18T14:23:32Z

just for curiosity: please explain why the branch name is important?
just because you are creating two diffent PRs or is it somthing else?

tony · 2017-12-18T14:45:16Z

just for curiosity: please explain why the branch name is important?

It's best practice to pick a branch name if you intend on merging another branch on top of it.

In this specific case, there are conflicts and a need to rebase rtfd's master branch. But this PR uses the master branch. That adds a lot of ambiguity. I am switching between branches with 4 different remotes (rtfd, develtech, tony, and yours). Each have "master" branch in a different spot.

just because you are creating two diffent PRs or is it somthing else?

Nope.

The only way to merge from a different branch is a new PR.

Technically, I could close this and make a new PR from your repo. If I did this, you would still get credit in the git history (you would even if I forked your branch and PR'd from my own repo). This is because your commits are still there. However, I also want you to keep the credit for opening the PR, too. That's why I asked you to do it.

The branches:

django-1.11 is where I want to do work. I'm going to be doing rebasing (and probably resolving a lot of conflicts) and rebasing out (permanently removing) changes that were edited out in history.

django-1.11-backup is a pristine backup of this PR.

kakulukia · 2017-12-18T15:15:24Z

please go ahead and do whats neccessary to fix the problem - sweet that you care about my credits, but if something gets missing i will still be able to sleep at nights, tho :)

tony · 2017-12-18T15:41:52Z

Old commits: 9190111 ... 6250cd4

New rebase against master: 20cc779 ... 761e95e

tony · 2017-12-18T15:46:20Z

Next, going to squash. Backing up branch at https://github.com/kakulukia/readthedocs.org/blob/django-1.11-beforesquash/.travis.yml

See also readthedocs#3297

tony · 2017-12-18T16:15:03Z

OK, rebased against master (9e7bcb7), removed Pipfile stuff, and squashed.

Also, ff7acc8 seemed to fix the GDAL package issue.

So there are some remaining bugs cropping up:

API Tests, sync_versions, and project_views may be having some issues related to render()

https://travis-ci.org/rtfd/readthedocs.org/jobs/318152474

Linting: Separate from this PR, see lint prospector task failing #3414 and AttributeError: 'NoneType' object has no attribute 'pattern' pylint-dev/pylint-django#108

https://travis-ci.org/rtfd/readthedocs.org/jobs/318152478

lint runtests: commands[0] | prospector --profile-path=/home/travis/build/rtfd/readthedocs.org --profile=prospector-more --die-on-tool-error

Traceback (most recent call last):

  File "/home/travis/build/rtfd/readthedocs.org/.tox/lint/bin/prospector", line 11, in <module>

    sys.exit(main())

  File "/home/travis/build/rtfd/readthedocs.org/.tox/lint/lib/python2.7/site-packages/prospector/run.py", line 165, in main

    prospector.execute()

  File "/home/travis/build/rtfd/readthedocs.org/.tox/lint/lib/python2.7/site-packages/prospector/run.py", line 53, in execute

    for tool in self.config.get_tools(found_files):

  File "/home/travis/build/rtfd/readthedocs.org/.tox/lint/lib/python2.7/site-packages/prospector/config/__init__.py", line 45, in get_tools

    config_result = tool.configure(self, found_files)

  File "/home/travis/build/rtfd/readthedocs.org/.tox/lint/lib/python2.7/site-packages/prospector/tools/pylint/__init__.py", line 174, in configure

    self._prospector_configure(prospector_config, linter)

  File "/home/travis/build/rtfd/readthedocs.org/.tox/lint/lib/python2.7/site-packages/prospector/tools/pylint/__init__.py", line 34, in _prospector_configure

    linter.load_plugin_modules(['pylint_django'])

  File "/home/travis/build/rtfd/readthedocs.org/.tox/lint/lib/python2.7/site-packages/pylint/lint.py", line 495, in load_plugin_modules

    module.register(self)

  File "/home/travis/build/rtfd/readthedocs.org/.tox/lint/lib/python2.7/site-packages/pylint_django/plugin.py", line 22, in register

    start = name_checker.config.const_rgx.pattern[:-2]

AttributeError: 'NoneType' object has no attribute 'pattern'

ERROR: InvocationError: '/home/travis/build/rtfd/readthedocs.org/.tox/lint/bin/prospector --profile-path=/home/travis/build/rtfd/readthedocs.org --profile=prospector-more --die-on-tool-error'

___________________________________ summary ____________________________________

ERROR:   lint: commands failed

tony

7b81f84

tony · 2017-12-18T16:30:48Z

readthedocs/settings/base.py

+            'BACKEND': 'django.template.backends.django.DjangoTemplates',
+            'DIRS': [TEMPLATE_ROOT],
+            'OPTIONS': {
+                'debug': True,


Should be 'debug': DEBUG,

tony · 2017-12-18T16:40:45Z

Judging by the breaking of API endpoints, there is more at play between this and pagination.

self = <WSGIRequest: GET '/projects/django-kong/builds/'>, suffix = ''


    def get_page(self, suffix):

        """

        A function which will be monkeypatched onto the request to get the current

        integer representing the current page.

        """

        try:

>           return int(self.REQUEST['page%s' % suffix])

E           AttributeError: 'WSGIRequest' object has no attribute 'REQUEST'


../.tox/py36/lib/python3.6/site-packages/linaro_django_pagination/middleware.py:38: AttributeError

There is no more self.REQUEST, there is self.GET and self.POST.

zyga/django-pagination#41 (comment)

Anndd, the project is no longer maintained. So we're blocked by that.

tony · 2017-12-19T14:07:44Z

This is becoming a lot more complicated.

pagination

I could fork and fix django-pagination. But I can't maintain it.

I'm also not quite sure why django's pagination isn't being used.

tastypie error

self = <readthedocs.api.base.ProjectResource object at 0x7ffbd6704310>

bundle = <Bundle for obj: '' and with data: '{}'>

kwargs = {'slug': 'django-kong'}


    def obj_get(self, bundle, **kwargs):

        """

            A ORM-specific implementation of ``obj_get``.

    

            Takes optional ``kwargs``, which are used to narrow the query to find

            the instance.

            """

        # prevents FieldError when looking up nested resources containing extra data

>       field_names = self._meta.object_class._meta.get_all_field_names()

E       AttributeError: 'Options' object has no attribute 'get_all_field_names'


../.tox/py27/lib/python2.7/site-packages/tastypie/resources.py:2116: AttributeError

----------------------------- Captured stderr call -----------------------------

may be fixed by upgrading to django-tastypie==0.14.0

tony · 2017-12-19T14:12:37Z

This requires too much work for me at the moment. I'd probably end up doing my own PR because it's more than switching out a few packages/updating functions, there are dependencies down the line that need phasing out. Too much on my plate ATM.

If someone else wants to take this up, reply here and make a fresh PR

kakulukia · 2017-12-19T16:24:01Z

@tony do you know why there is a need for custom rather than django builtin pagination? maybe it can very well be removed from the dependencies because its just a relic from old times?

tony · 2017-12-19T16:29:54Z

@kakulukia And I should have asked you first, if you want to finish this PR. Or let someone fork it / do their own.

@tony do you know why there is a need for custom rather than django builtin pagination? maybe it can very well be removed from the dependencies because its just a relic from old times?

This is a big codebase. Figuring out whether a third party pagination plugin was cargo-culted in or is now redundant would take time. I have more urgent thing pressing on me ATM (like paying bills 😄)

kakulukia · 2017-12-19T16:44:57Z

well, as far as i tested my latest commit i didnt have a problem with the pagination module.
I will have another look next week to maybe figure out whats broken after you fixed the gdal thing. :)

Could you add a help wanted label?

stsewd · 2018-04-28T21:55:05Z

I think is better to follow the Django recommendations for upgrading to a new version https://docs.djangoproject.com/en/2.0/howto/upgrade-version/

Upgrade from each version till the current one (2.0) instead of upgrading to 2.0 in one step.

stsewd · 2018-08-30T05:33:45Z

Closing this in favor of #4319, which do the upgrade in a more granular way (first to 1.10), thanks to all people here, sorry that this wasn't merged, maybe you can help to review the other PR, or updating to 2.0 later.

kakulukia mentioned this pull request Nov 23, 2017

django.contrib.gis.geos.error.GEOSException: Could not parse version info string "3.6.2-CAPI-1.10.2 4d2925d6" #3297

Closed

humitos requested changes Nov 24, 2017

View reviewed changes

humitos changed the title ~~fixed https://github.com/rtfd/readthedocs.org/issues/3297#issuecomment-346395199~~ Upgrade all packages, including Django because #3297 Nov 24, 2017

RichardLitt added the PR: work in progress Pull request is not ready for full review label Nov 27, 2017

RichardLitt added the Status: blocked Issue is blocked on another issue label Dec 17, 2017

tony force-pushed the master branch from 7cc235a to 828a8de Compare December 18, 2017 15:41

kakulukia and others added 2 commits December 18, 2017 09:53

Update Django to 1.11

0c744b3

See also readthedocs#3297

try adding gdal packages to travis

ff7acc8

tony force-pushed the master branch from 828a8de to ff7acc8 Compare December 18, 2017 15:56

tony mentioned this pull request Dec 18, 2017

lint prospector task failing #3414

Closed

tony approved these changes Dec 18, 2017

View reviewed changes

nara1823 approved these changes Jan 27, 2018

View reviewed changes

kakulukia changed the title ~~Upgrade all packages, including Django because #3297~~ Upgrade Django because #3297 Jan 29, 2018

xrmx mentioned this pull request Feb 22, 2018

Upgrade all packages using pur tool #2916

Merged

stsewd mentioned this pull request May 31, 2018

Meta: Upgrade Django to 1.10 #4174

Closed

5 tasks

stsewd closed this Aug 30, 2018

Upgrade Django because #3297 #3305

Upgrade Django because #3297 #3305

Conversation

kakulukia commented Nov 23, 2017

kakulukia commented Nov 23, 2017

humitos left a comment

Choose a reason for hiding this comment

humitos Nov 24, 2017

Choose a reason for hiding this comment

tony Dec 18, 2017

Choose a reason for hiding this comment

kakulukia Dec 18, 2017

Choose a reason for hiding this comment

humitos Nov 24, 2017

Choose a reason for hiding this comment

humitos Nov 24, 2017

Choose a reason for hiding this comment

kakulukia commented Nov 24, 2017

kakulukia commented Nov 24, 2017 • edited Loading

humitos commented Nov 24, 2017

kakulukia commented Nov 24, 2017

kakulukia commented Nov 24, 2017

kakulukia commented Nov 24, 2017

kakulukia commented Nov 25, 2017

tony commented Dec 17, 2017

tony commented Dec 17, 2017

kakulukia commented Dec 17, 2017

kakulukia commented Dec 18, 2017

tony commented Dec 18, 2017

tony commented Dec 18, 2017 • edited Loading

tony commented Dec 18, 2017 • edited Loading

tony commented Dec 18, 2017

kakulukia commented Dec 18, 2017

tony commented Dec 18, 2017 • edited Loading

kakulukia commented Dec 18, 2017

tony commented Dec 18, 2017

tony commented Dec 18, 2017

tony commented Dec 18, 2017 • edited Loading

tony left a comment

Choose a reason for hiding this comment

tony Dec 18, 2017

Choose a reason for hiding this comment

tony commented Dec 18, 2017 • edited Loading

tony commented Dec 19, 2017

pagination

tastypie error

tony commented Dec 19, 2017

kakulukia commented Dec 19, 2017

tony commented Dec 19, 2017 • edited Loading

kakulukia commented Dec 19, 2017

stsewd commented Apr 28, 2018

stsewd commented Aug 30, 2018

kakulukia commented Nov 24, 2017 •

edited

Loading

tony commented Dec 18, 2017 •

edited

Loading

tony commented Dec 18, 2017 •

edited

Loading

tony commented Dec 18, 2017 •

edited

Loading

tony commented Dec 18, 2017 •

edited

Loading

tony commented Dec 18, 2017 •

edited

Loading

tony commented Dec 19, 2017 •

edited

Loading