Add blacklist, here exclude list, of packages not to install #186

CodeHeeler · 2018-06-19T12:36:26Z

No description provided.

pep8speaks · 2018-06-19T12:36:28Z

Hello @CodeHeeler! Thanks for submitting the PR.

In the file pulp_python/app/models.py, following are the PEP8 issues :

Line 85:32: E128 continuation line under-indented for visual indent
Line 86:32: E128 continuation line under-indented for visual indent
Line 87:32: E128 continuation line under-indented for visual indent

In the file pulp_python/app/tasks/sync.py, following are the PEP8 issues :

Line 144:1: W293 blank line contains whitespace

pulpbot · 2018-06-19T12:47:12Z

Can one of the admins verify this patch?

pulpbot · 2018-06-19T12:47:31Z

Can one of the admins verify this patch?

werwty · 2018-06-19T13:34:46Z

@CodeHeeler please rebase to latest 3.0-dev

Currently if includes isn't set, it defaults to excludes, so the following would actually not sync anything.

http POST $BASE_ADDR/pulp/api/v3/remotes/python/ \
    name='bar' \
    url='https://pypi.org/' \
    projects:='[{"name": "django", "version_specifier":"~=2.0"}]'

I was hoping for something more like:

http POST $BASE_ADDR/pulp/api/v3/remotes/python/ \
    name='bar' \
    url='https://pypi.org/' \
    includes:='[{"name": "django", "version_specifier":"~=2.0"}]' \
    excludes:='[{"name": "django", "version_specifier":"=2.0.1"}]'

Which can be done by

1.) creating two separate models, each subclassing ProjectSpecifier: class IncludeSpecifiers(ProjectSpecifier): class ExcludeSpecifiers(ProjectSpecifier):

or
2.) keeping a boolean on the ProjectSpecifier model and having the Remote serializer filter the boolean for two different fields. So an additional boolean to models.py:

class ProjectSpecifier(Model):
    name = models.TextField()
    version_specifier = models.TextField(blank=True, default="")
    remote = models.ForeignKey("PythonRemote",
                               related_name="projects",
                               related_query_name="projectspecifier",
                               on_delete=models.CASCADE)
    include = models.BooleanField()

and in serializers.py (just pseudocode, might not actually work this way):

class PythonRemoteSerializer(core_serializers.RemoteSerializer):
    include = ProjectSpecifierSerializer(
        required=False,
        many=True,
        instance=ProjectSpecifier.objects.filter(include=True, remote=)
    )
    exclude = ProjectSpecifierSerializer(
        required=False,
        many=True,
        instance=ProjectSpecifier.objects.filter(include=False, remote=)
    )

Here's an example on how serializer filtering can be done: https://stackoverflow.com/questions/28309507/django-rest-framework-filtering-for-serializer-field

This would also necessitate amending the RemoteSerializer create() and update() functions. This is more complex, but cleaner overall.

dralley · 2018-07-12T20:20:47Z

pulp_python/app/tasks/sync.py

+                        else:
+                            for type, digest in package['digests'].items():
+                                if digests.filter(type=type, digest=digest).exists():
+                                    remote_units.append(parse_metadata(metadata['info'],


re-indent this across multiple lines as seen elsewhere in the codebase

dralley · 2018-07-12T20:21:21Z

pulp_python/app/models.py

-        related_query_name="projectspecifier",
-        on_delete=models.CASCADE
-    )
+    remote = models.ForeignKey("PythonRemote",


Reindent this to be as it was previously

dralley · 2018-07-12T20:23:35Z

pulp_python/app/models.py

@@ -167,3 +167,9 @@ class PythonRemote(Remote):
    """

    TYPE = 'python'
+
+    def includes_specifiers(self):


Should these be decorated with @property?

Also please add docstrings

dralley · 2018-07-16T19:08:18Z

pulp_python/app/tasks/sync.py

@@ -56,10 +56,11 @@ def sync(remote_pk, repository_pk):
                .format(repository=repository.name, remote=remote.name)
            )

-            project_specifiers = python_models.ProjectSpecifier.objects.filter(remote=remote).all()
+            includes = python_models.ProjectSpecifier.objects.filter(remote=remote, include=True).all()
+            excludes = python_models.ProjectSpecifier.objects.filter(remote=remote, include=False).all()


I don't think the .all() is necessary here.

dralley · 2018-07-16T19:10:19Z

pulp_python/app/tasks/sync.py

-                # If neither specifiers nor digests have been set, then we should add the unit
-                if not project.version_specifier and not digests.exists():
+                if project.name in excludes:
+                    whatnottosync=excludes[project.name]


whatnottosync is a little unweildy, maybe call it excluded_pkg?

Equals sign should have spaces on either side (PEP8).

dralley · 2018-07-16T19:27:03Z

pulp_python/app/tasks/sync.py

    """
    Fetch metadata for content units matching project specifiers from the remote.

    Args:
        project_specifiers (dict): Information about a project and which versions of a project
            to filter

+    These units are the difference between the include list and the exclude list.
+


Docstring needs to be fixed with the new arguments, still lists project_specifiers.

This one sentence belongs underneath the first one.

dralley · 2018-07-16T19:33:16Z

See Travis logs for failing PEP8 checks

dralley · 2018-08-06T14:16:41Z

@CodeHeeler could you rebase on current master?

Now packages are not only checked against a whitelist/include list but also against an exclude list when syncing. fixes #3672 https://pulp.plan.io/issues/3672

dralley · 2018-08-07T15:45:14Z

pulp_python/tests/functional/utils.py

-def gen_remote(url=PYTHON_FIXTURES_URL, projects=PYTHON_XS_PROJECT_SPECIFIER, **kwargs):
+def gen_remote(
+        url=PYTHON_FIXTURES_URL,
+        projects=PYTHON_XS_PROJECT_SPECIFIER,


remove projects here and below

and do a search to replace gen_remote(.., projects=..) with gen_remote(.., includes=..)

This reverts commit 0ff205a.

dralley · 2018-08-07T18:19:41Z

There will also need to be some documentation updates to cover the new names. Preferably an example of using Excludes in practice, but at minimum changing e.g. "projects" to "includes" so that the existing examples will still work.

If during the process of testing this you also feel like changing the example urls from the old UUID form to the new integer ID form, that would also be appreciated :) but you don't have to.

CodeHeeler · 2018-08-07T18:40:19Z

Cool, thanks for pointing that out!

dralley · 2018-08-08T19:23:00Z

Going to remake this PR on top of the Declarative Version changes, once those are merged.

dralley added the Work In Progress label Jun 19, 2018

werwty added the 3.0 label Jun 19, 2018

CodeHeeler changed the title ~~Issue 3672~~ Add blacklist, here exclude list, of packages not to install Jun 19, 2018

CodeHeeler force-pushed the issue-3672 branch 2 times, most recently from 9a3b4f9 to d4dbe34 Compare June 26, 2018 18:16

werwty changed the base branch from 3.0-dev to master July 2, 2018 18:12

werwty force-pushed the master branch from a41e2a5 to b07999d Compare July 2, 2018 18:13

CodeHeeler force-pushed the issue-3672 branch from d4dbe34 to da8e280 Compare July 3, 2018 13:28

dralley reviewed Jul 12, 2018

View reviewed changes

dralley reviewed Jul 16, 2018

View reviewed changes

CodeHeeler force-pushed the issue-3672 branch 4 times, most recently from fbb5011 to 81c643f Compare August 3, 2018 16:57

CodeHeeler force-pushed the issue-3672 branch 6 times, most recently from dd89b85 to 8441126 Compare August 6, 2018 19:56

Add blacklist, here exclude list, of packages not to install

92904c5

Now packages are not only checked against a whitelist/include list but also against an exclude list when syncing. fixes #3672 https://pulp.plan.io/issues/3672

CodeHeeler force-pushed the issue-3672 branch 5 times, most recently from 365624a to 5845656 Compare August 7, 2018 15:37

dralley reviewed Aug 7, 2018

View reviewed changes

Revert "Filters inherit from our class BaseFilterSet"

49a2120

This reverts commit 0ff205a.

CodeHeeler force-pushed the issue-3672 branch from 5845656 to 49a2120 Compare August 7, 2018 15:51

dralley closed this Aug 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add blacklist, here exclude list, of packages not to install #186

Add blacklist, here exclude list, of packages not to install #186

CodeHeeler commented Jun 19, 2018

pep8speaks commented Jun 19, 2018

pulpbot commented Jun 19, 2018

pulpbot commented Jun 19, 2018

werwty commented Jun 19, 2018 •

edited

Loading

dralley Jul 12, 2018

dralley Jul 12, 2018

dralley Jul 12, 2018

dralley Jul 16, 2018

dralley Jul 16, 2018 •

edited

Loading

dralley Jul 16, 2018

dralley commented Jul 16, 2018

dralley commented Aug 6, 2018

dralley Aug 7, 2018

dralley Aug 7, 2018

dralley commented Aug 7, 2018

CodeHeeler commented Aug 7, 2018

dralley commented Aug 8, 2018

Add blacklist, here exclude list, of packages not to install #186

Add blacklist, here exclude list, of packages not to install #186

Conversation

CodeHeeler commented Jun 19, 2018

pep8speaks commented Jun 19, 2018

pulpbot commented Jun 19, 2018

pulpbot commented Jun 19, 2018

werwty commented Jun 19, 2018 • edited Loading

dralley Jul 12, 2018

Choose a reason for hiding this comment

dralley Jul 12, 2018

Choose a reason for hiding this comment

dralley Jul 12, 2018

Choose a reason for hiding this comment

dralley Jul 16, 2018

Choose a reason for hiding this comment

dralley Jul 16, 2018 • edited Loading

Choose a reason for hiding this comment

dralley Jul 16, 2018

Choose a reason for hiding this comment

dralley commented Jul 16, 2018

dralley commented Aug 6, 2018

dralley Aug 7, 2018

Choose a reason for hiding this comment

dralley Aug 7, 2018

Choose a reason for hiding this comment

dralley commented Aug 7, 2018

CodeHeeler commented Aug 7, 2018

dralley commented Aug 8, 2018

werwty commented Jun 19, 2018 •

edited

Loading

dralley Jul 16, 2018 •

edited

Loading