Project

Profile

Help

Task #2878

Enable nitpicky mode for docs building

Added by ttereshc over 2 years ago. Updated 6 months ago.

Status:
NEW
Priority:
Normal
Assignee:
-
Category:
-
Sprint/Milestone:
-
Start date:
Due date:
% Done:

0%

Platform Release:
Blocks Release:
Backwards Incompatible:
No
Groomed:
Yes
Sprint Candidate:
Tags:
Documentation
QA Contact:
Complexity:
Smash Test:
Verified:
No
Verification Required:
No
Sprint:

Description

Enable nitpicky mode in the Makefile for docs building, so all the missing references in the docs will be seen and in combination with -W option treated as errors and fail a build.
Fix all the warnings after turning nitpicky mode on, currently there are plenty of them.

NOTE: sphinx-build -n -W doesn't treat warnings which come from nitpicky mode as errors, Sphinx upstream issue is filed. So if currently warning doesn't result in an error and build succeeds, please, fix it anyway.


Checklist

History

#1 Updated by bizhang over 2 years ago

  • Groomed changed from No to Yes
  • Tags Documentation added

#2 Updated by ttereshc over 2 years ago

  • Sprint Candidate changed from No to Yes

#3 Updated by mhrivnak about 2 years ago

  • Sprint/Milestone set to 42

#4 Updated by ttereshc about 2 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to ttereshc

#6 Updated by ttereshc about 2 years ago

I ran into several issues with this task. And the main ones are:
  1. Class attributes in docstrings for models.
    They are documented under Fields to avoid duplicate cross-reference targets
    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.
    In platform/pulpcore/app/models/content.py (no complains because Artifact is defined here):
    class Artifact(Model):
        """ 
        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.
        """ 
        file = models.FileField(db_index=True, upload_to=storage_path, max_length=255)
        size = models.IntegerField(blank=False, null=False)
        md5 = models.CharField(max_length=32, blank=False, null=False, unique=False, db_index=True)
        sha1 = models.CharField(max_length=40, blank=False, null=False, unique=False, db_index=True)
        sha224 = models.CharField(max_length=56, blank=False, null=False, unique=False, db_index=True)
        sha256 = models.CharField(max_length=64, blank=False, null=False, unique=True, db_index=True)
        sha384 = models.CharField(max_length=96, blank=False, null=False, unique=True, db_index=True)
        sha512 = models.CharField(max_length=128, blank=False, null=False, unique=True, db_index=True)
    

    But because of platform/pulpcore/app/models/__init__.py:
    from .content import Artifact 
    

    For modules which import Artifact and which are autodoc'd plugin/pulpcore/plugin/models/__init__.py:
    from pulpcore.app.models import Artifact
    

    there will be warnings for class attributes:
    pulp: sphinx.transforms.post_transforms:WARNING: py:data reference target not found: sha512
    

    Take the explanation above as my observation, no deep investigation of the root cause was done.
    But I checked that making imports only from the module where model is defined (from pulpcore.app.models.content import Artifact) 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.
  2. When class attribute is an imported object
    from pulpcore.app.serializers import ArtifactSerializer
    
    class ArtifactViewSet(NamedModelViewSet):
        endpoint_name = 'artifacts'
        queryset = Artifact.objects.all()
        serializer_class = ArtifactSerializer
        filter_class = ArtifactFilter
    

    pulp: sphinx.transforms.post_transforms:WARNING: py:class reference target not found: ArtifactSerializer
    
  3. So far I was not able to find any more or less easy/acceptable ways to ignore or overcome issues above.
    E.g. nitpicky_ignore is expected to have every single target (aka every class attribute) in its list, no wildcards or regexp for that.
  4. Sphinx upstream issue is not resolved and it does not look like it will be any time soon.

Any ideas about how to solve described issues are welcome.

Potential next step: create a reproducer and ask on #sphinx-doc for suggestions.

#7 Updated by bmbouter about 2 years ago

Thanks @ttereshc 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.

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 this or this. So my recommendation is to stop importing code from places other than where they are defined. That will resolve all of these issues.

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:

Option 1: We could use nitpicky_ignore for each one and I think that would be ok.

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 nitpicky_ignore.

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.

#8 Updated by ttereshc about 2 years ago

  • Sprint/Milestone deleted (42)

Postponed to be solved after Plugin API Alpha release

#9 Updated by ttereshc about 2 years ago

  • Status changed from ASSIGNED to NEW
  • Assignee deleted (ttereshc)

#10 Updated by rchan over 1 year ago

  • Sprint Candidate deleted (Yes)

#11 Updated by bmbouter 6 months ago

  • Tags deleted (Pulp 3)

Please register to edit this issue

Also available in: Atom PDF