https://pulp.plan.io/https://pulp.plan.io/favicon.ico2017-07-10T14:52:56ZPulpPulp - Task #2878: Enable nitpicky mode for docs buildinghttps://pulp.plan.io/issues/2878?journal_id=206832017-07-10T14:52:56Zbizhangbizhang@redhat.com
<ul><li><strong>Groomed</strong> changed from <i>No</i> to <i>Yes</i></li><li><strong>Tags</strong> <i>Documentation</i> added</li></ul> Pulp - Task #2878: Enable nitpicky mode for docs buildinghttps://pulp.plan.io/issues/2878?journal_id=206842017-07-10T14:54:41Zttereshcttereshc@redhat.com
<ul><li><strong>Sprint Candidate</strong> changed from <i>No</i> to <i>Yes</i></li></ul> Pulp - Task #2878: Enable nitpicky mode for docs buildinghttps://pulp.plan.io/issues/2878?journal_id=210692017-07-20T14:40:48Zmhrivnakmhrivnak@redhat.com
<ul><li><strong>Sprint/Milestone</strong> set to <i>42</i></li></ul> Pulp - Task #2878: Enable nitpicky mode for docs buildinghttps://pulp.plan.io/issues/2878?journal_id=212822017-07-31T12:39:57Zttereshcttereshc@redhat.com
<ul><li><strong>Status</strong> changed from <i>NEW</i> to <i>ASSIGNED</i></li><li><strong>Assignee</strong> set to <i>ttereshc</i></li></ul> Pulp - Task #2878: Enable nitpicky mode for docs buildinghttps://pulp.plan.io/issues/2878?journal_id=213832017-08-02T23:46:35Zttereshcttereshc@redhat.com
<ul></ul><p>WIP PR: <a href="https://github.com/pulp/pulp/pull/3113" class="external">https://github.com/pulp/pulp/pull/3113</a></p> Pulp - Task #2878: Enable nitpicky mode for docs buildinghttps://pulp.plan.io/issues/2878?journal_id=214052017-08-04T20:50:31Zttereshcttereshc@redhat.com
<ul></ul><p>I ran into several issues with this task. And the main ones are:</p>
<ol>
<li>
<p><strong>Class attributes in docstrings for models.</strong><br>
They are documented under <a href="https://github.com/pulp/pulp/blob/bbcdb7c378d0784f0bf12a5e870b1eb06acb3ec1/docs/extensions/napoleon_django/docstring.py#L24" class="external"><code>Fields</code> to avoid duplicate cross-reference targets</a><br>
In a combination with the way how we do imports (not directly from the module were model is defined) reference to the object is not found.<br>
In <code>platform/pulpcore/app/models/content.py</code> (no complains because <code>Artifact</code> is defined here):</p>
<pre><code class="python syntaxhl" data-language="python"><span class="k">class</span> <span class="nc">Artifact</span><span class="p">(</span><span class="n">Model</span><span class="p">):</span>
<span class="s">"""
A file associated with a piece of content.
Fields:
file (~django.db.models.FileField): The stored file.
size (~django.db.models.IntegerField): The size of the file in bytes.
md5 (~django.db.models.CharField): The MD5 checksum of the file.
sha1 (~django.db.models.CharField): The SHA-1 checksum of the file.
sha224 (~django.db.models.CharField): The SHA-224 checksum of the file.
sha256 (~django.db.models.CharField): The SHA-256 checksum of the file.
sha384 (~django.db.models.CharField): The SHA-384 checksum of the file.
sha512 (~django.db.models.CharField): The SHA-512 checksum of the file.
"""</span>
<span class="nb">file</span> <span class="o">=</span> <span class="n">models</span><span class="p">.</span><span class="n">FileField</span><span class="p">(</span><span class="n">db_index</span><span class="o">=</span><span class="bp">True</span><span class="p">,</span> <span class="n">upload_to</span><span class="o">=</span><span class="n">storage_path</span><span class="p">,</span> <span class="n">max_length</span><span class="o">=</span><span class="mi">255</span><span class="p">)</span>
<span class="n">size</span> <span class="o">=</span> <span class="n">models</span><span class="p">.</span><span class="n">IntegerField</span><span class="p">(</span><span class="n">blank</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">null</span><span class="o">=</span><span class="bp">False</span><span class="p">)</span>
<span class="n">md5</span> <span class="o">=</span> <span class="n">models</span><span class="p">.</span><span class="n">CharField</span><span class="p">(</span><span class="n">max_length</span><span class="o">=</span><span class="mi">32</span><span class="p">,</span> <span class="n">blank</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">null</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">unique</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">db_index</span><span class="o">=</span><span class="bp">True</span><span class="p">)</span>
<span class="n">sha1</span> <span class="o">=</span> <span class="n">models</span><span class="p">.</span><span class="n">CharField</span><span class="p">(</span><span class="n">max_length</span><span class="o">=</span><span class="mi">40</span><span class="p">,</span> <span class="n">blank</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">null</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">unique</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">db_index</span><span class="o">=</span><span class="bp">True</span><span class="p">)</span>
<span class="n">sha224</span> <span class="o">=</span> <span class="n">models</span><span class="p">.</span><span class="n">CharField</span><span class="p">(</span><span class="n">max_length</span><span class="o">=</span><span class="mi">56</span><span class="p">,</span> <span class="n">blank</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">null</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">unique</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">db_index</span><span class="o">=</span><span class="bp">True</span><span class="p">)</span>
<span class="n">sha256</span> <span class="o">=</span> <span class="n">models</span><span class="p">.</span><span class="n">CharField</span><span class="p">(</span><span class="n">max_length</span><span class="o">=</span><span class="mi">64</span><span class="p">,</span> <span class="n">blank</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">null</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">unique</span><span class="o">=</span><span class="bp">True</span><span class="p">,</span> <span class="n">db_index</span><span class="o">=</span><span class="bp">True</span><span class="p">)</span>
<span class="n">sha384</span> <span class="o">=</span> <span class="n">models</span><span class="p">.</span><span class="n">CharField</span><span class="p">(</span><span class="n">max_length</span><span class="o">=</span><span class="mi">96</span><span class="p">,</span> <span class="n">blank</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">null</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">unique</span><span class="o">=</span><span class="bp">True</span><span class="p">,</span> <span class="n">db_index</span><span class="o">=</span><span class="bp">True</span><span class="p">)</span>
<span class="n">sha512</span> <span class="o">=</span> <span class="n">models</span><span class="p">.</span><span class="n">CharField</span><span class="p">(</span><span class="n">max_length</span><span class="o">=</span><span class="mi">128</span><span class="p">,</span> <span class="n">blank</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">null</span><span class="o">=</span><span class="bp">False</span><span class="p">,</span> <span class="n">unique</span><span class="o">=</span><span class="bp">True</span><span class="p">,</span> <span class="n">db_index</span><span class="o">=</span><span class="bp">True</span><span class="p">)</span>
</code></pre>
<p>But because of <code>platform/pulpcore/app/models/__init__.py</code>:</p>
<pre><code class="python syntaxhl" data-language="python"><span class="kn">from</span> <span class="nn">.content</span> <span class="kn">import</span> <span class="n">Artifact</span>
</code></pre>
<p>For modules which import <code>Artifact</code> and which are autodoc'd <code>plugin/pulpcore/plugin/models/__init__.py</code>:</p>
<pre><code class="python syntaxhl" data-language="python"><span class="kn">from</span> <span class="nn">pulpcore.app.models</span> <span class="kn">import</span> <span class="n">Artifact</span>
</code></pre>
<p>there will be warnings for class attributes:</p>
<pre><code>pulp: sphinx.transforms.post_transforms:WARNING: py:data reference target not found: sha512
</code></pre>
<p>Take the explanation above as my observation, no deep investigation of the root cause was done.<br>
But I checked that making imports only from the module where model is defined (<code>from pulpcore.app.models.content import Artifact</code>) solves this docs issue. It's not a suggestion for a solution, just a way to show/prove where the problem is or at least what causes it.</p>
</li>
<li>
<p><strong>When class attribute is an imported object</strong></p>
<pre><code class="python syntaxhl" data-language="python"><span class="kn">from</span> <span class="nn">pulpcore.app.serializers</span> <span class="kn">import</span> <span class="n">ArtifactSerializer</span>
<span class="k">class</span> <span class="nc">ArtifactViewSet</span><span class="p">(</span><span class="n">NamedModelViewSet</span><span class="p">):</span>
<span class="n">endpoint_name</span> <span class="o">=</span> <span class="s">'artifacts'</span>
<span class="n">queryset</span> <span class="o">=</span> <span class="n">Artifact</span><span class="p">.</span><span class="n">objects</span><span class="p">.</span><span class="nb">all</span><span class="p">()</span>
<span class="n">serializer_class</span> <span class="o">=</span> <span class="n">ArtifactSerializer</span>
<span class="n">filter_class</span> <span class="o">=</span> <span class="n">ArtifactFilter</span>
</code></pre>
<pre><code>pulp: sphinx.transforms.post_transforms:WARNING: py:class reference target not found: ArtifactSerializer
</code></pre>
</li>
<li>
<p><strong>So far I was not able to find any more or less easy/acceptable ways to ignore or overcome issues above.</strong><br>
E.g. <code>nitpicky_ignore</code> is expected to have every single target (aka every class attribute) in its list, no wildcards or regexp for that.</p>
</li>
<li>
<p><strong><a href="https://github.com/sphinx-doc/sphinx/issues/3919" class="external">Sphinx upstream issue</a> is not resolved and it does not look like it will be any time soon.</strong></p>
</li>
</ol>
<p>Any ideas about how to solve described issues are welcome.</p>
<p>Potential next step: create a reproducer and ask on #sphinx-doc for suggestions.</p> Pulp - Task #2878: Enable nitpicky mode for docs buildinghttps://pulp.plan.io/issues/2878?journal_id=214212017-08-07T16:55:19Zbmbouterbmbouter@redhat.com
<ul></ul><p>Thanks <a class="user active" href="https://pulp.plan.io/users/281">ttereshc</a> for the detailed analysis. I think the best way to resolve this is to avoid triggering the problems. I believe the two causes of these issues are actually things we shouldn't be doing.</p>
<p>Over the years I've learned that writing code in one place and then importing it somewhere where you don't plan to use it not a good design pattern. I've been told most circular import problems are a result of this type of code. One of the most common resolutions to those issues is to "guard imports" so they occur at runtime instead. We are already doing this bad practice in places like <a href="https://github.com/pulp/pulp/blob/e5986173d1617fcadb520c84b78691982cce2218/platform/pulpcore/tasking/celery_app.py#L160" class="external">this</a> or <a href="https://github.com/pulp/pulp/blob/e5986173d1617fcadb520c84b78691982cce2218/platform/pulpcore/tasking/celery_app.py#L206" class="external">this</a>. So my recommendation is to stop importing code from places other than where they are defined. That will resolve all of these issues.</p>
<p>Regarding the class attribute problem. DRF requires us to set things as class attributes so we probably can't easily workaround that one. Here are some ideas:</p>
<p>Option 1: We could use <code>nitpicky_ignore</code> for each one and I think that would be ok.</p>
<p>Option 2: We could also do less autodoc of those things. I only thing we need to autodoc the plugin API. I expect pulp developers can read the code so that could also significantly reduce the amount of whitelisting we would have to do with <code>nitpicky_ignore</code>.</p>
<p>Option 3: We could consolidate all of the serializers into the same module so that we won't have to avoid assigning a class attribute from something that was imported.</p> Pulp - Task #2878: Enable nitpicky mode for docs buildinghttps://pulp.plan.io/issues/2878?journal_id=214932017-08-10T14:08:13Zttereshcttereshc@redhat.com
<ul><li><strong>Sprint/Milestone</strong> deleted (<del><i>42</i></del>)</li></ul><p>Postponed to be solved after Plugin API Alpha release</p> Pulp - Task #2878: Enable nitpicky mode for docs buildinghttps://pulp.plan.io/issues/2878?journal_id=215222017-08-10T21:07:38Zttereshcttereshc@redhat.com
<ul><li><strong>Status</strong> changed from <i>ASSIGNED</i> to <i>NEW</i></li><li><strong>Assignee</strong> deleted (<del><i>ttereshc</i></del>)</li></ul><p>WIP PR: <a href="https://github.com/pulp/pulp/pull/3113" class="external">https://github.com/pulp/pulp/pull/3113</a></p> Pulp - Task #2878: Enable nitpicky mode for docs buildinghttps://pulp.plan.io/issues/2878?journal_id=244822018-02-09T15:10:17Zrchan
<ul><li><strong>Sprint Candidate</strong> deleted (<del><i>Yes</i></del>)</li></ul> Pulp - Task #2878: Enable nitpicky mode for docs buildinghttps://pulp.plan.io/issues/2878?journal_id=429032019-04-26T20:38:23Zbmbouterbmbouter@redhat.com
<ul><li><strong>Tags</strong> deleted (<del><i>Pulp 3</i></del>)</li></ul> Pulp - Task #2878: Enable nitpicky mode for docs buildinghttps://pulp.plan.io/issues/2878?journal_id=616392020-08-31T14:40:24Zdaviddavis
<ul><li><strong>Status</strong> changed from <i>NEW</i> to <i>CLOSED - WONTFIX</i></li><li><strong>Sprint Candidate</strong> set to <i>No</i></li></ul>