API Changes #133

dralley · 2018-11-27T20:23:56Z

Make changes necessary to support the API changes brought over from the "single-table-content" concept, where content-in-repository-version filtering is done inside the content app instead of on special endpoints on a repository version.

dralley · 2018-11-27T20:26:11Z

pulp_file/tests/functional/api/test_crud_content_unit.py

@@ -132,7 +132,7 @@ def test_all(self):
        sync(cfg, remote, repo)

        repo = client.get(repo['_href'])
-        content = get_content(repo)
+        content = get_file_content(repo)


Since these functions are just one-liners, we could just replace this with get_content()['file'], and I'm open to changing it to be that way if QE would prefer it.

Those changes will need to reflect in all plugins. Besides that, they are fine.

@rochacbruno, any other comments?

Minor update that a better idea would be to use get_content()[FILE_CONTENT_NAME] instead of a string literal, if we were to do things that way.

@kersommoura @dralley agreed, we can gradually replace the function with get_content()['file'] and keep the _file_ function with a DeprecationWarning for backwards compatibility.

OR, if this function exists only in this repo, remove it and change in all references

@rochacbruno That probably wouldn't be necessary since get_file_content() is defined inside the plugin, in pulp_file.tests.functional.utils, and isn't something in pulp-smash itself for which backwards compatibility would be an issue. But it's also something being added in this PR, so we can choose which way we want to go before this would get merged. Do you have a personal preference?

@rochacbruno I just saw your second comment but yes that is correct. Still, if you have a personal preference I can go ahead and change it to avoid creating more work later on. This PR will probably be open for a few more days since it goes along with a big pulpcore PR.

And since I'll need to copy this work over to each of the other plugins, I'd rather do things the preferred way to begin with :)

daviddavis · 2018-11-27T20:35:15Z

pulp_file/tests/functional/constants.py

@@ -9,7 +9,9 @@
    CONTENT_PATH
 )

-FILE_CONTENT_PATH = urljoin(CONTENT_PATH, 'file/files/')
+FILE_CONTENT_NAME = 'filecontent'


TBH, I am not a fan of this change. What's the reasoning behind using 'filecontent' instead of a namespaced endpoint?

Thanks for noticing it because it shouldn't be in this PR, accidental straggler.

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley · 2018-12-04T21:00:20Z

@pulp/qe

dralley · 2018-12-04T21:01:12Z

The smash tests pass locally, it just needs pulp/pulp-smash#1154 in order to work

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

Required PR: pulp/pulp-smash#1154 Required PR: pulp/pulp_file#133 [noissue]

Required PR: pulp/pulp#3774 Required PR: pulp/pulp-smash#1154 [noissue]

dkliban

LGTM once Travis is passing. I assume after pulpcore is merged.

dralley · 2018-12-17T17:35:22Z

@dkliban It'll be after this PR is merged pulp/pulp-smash#1154

Shorter and probably more efficient than fetching all the content data. [noissue]

Shorter and probably more efficient than fetching all the content data. Required PR: pulp/pulp-smash#1154 Required PR: pulp/pulp_file#133 [noissue]

nixocio · 2018-12-17T21:06:58Z

pulp_file/tests/functional/api/test_publish.py

+        """Create class-wide variables."""
+        cls.cfg = config.get_config()
+        cls.client = api.Client(cls.cfg, api.json_handler)
+


If we do not intend to reuse those objects they could be created inside the test method as before. Since there is just one test method.

nixocio · 2018-12-17T21:20:50Z

pulp_file/tests/functional/api/test_sync.py

-from pulp_file.tests.functional.utils import set_up_module as setUpModule  # noqa:F401
+from pulp_file.tests.functional.utils import (  # noqa:F401
+    gen_file_remote,
+    set_up_module as setUpModule


The # noqa:F401 is related to the setUpModule in the new format looks like both functions are part of this #noqa. I would rather keep them apart.

Required PR: pulp/pulp#3774 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added 3.0 labels Nov 27, 2018

dralley commented Nov 27, 2018

View reviewed changes

daviddavis reviewed Nov 27, 2018

View reviewed changes

dralley force-pushed the api-changes branch 5 times, most recently from adc3e43 to d98c48f Compare November 28, 2018 21:22

dralley removed the Work In Progress label Nov 28, 2018

dralley force-pushed the api-changes branch 2 times, most recently from f91b955 to 22d1d5c Compare December 4, 2018 18:01

dralley added a commit to dralley/pulp that referenced this pull request Dec 4, 2018

Update smash tests for new schema

91b63f2

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley force-pushed the api-changes branch from 22d1d5c to 71ea5f2 Compare December 4, 2018 18:04

dralley force-pushed the api-changes branch 2 times, most recently from 4b987c6 to ae12392 Compare December 4, 2018 22:16

dralley added a commit to dralley/pulp that referenced this pull request Dec 5, 2018

Update smash tests for new schema

2459fa8

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley force-pushed the api-changes branch from ae12392 to 7a490d7 Compare December 5, 2018 19:16

dralley added a commit to dralley/pulp that referenced this pull request Dec 6, 2018

Update smash tests for new schema

6ecd138

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added a commit to dralley/pulp that referenced this pull request Dec 10, 2018

Update smash tests for new schema

a04d74e

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added a commit to dralley/pulp that referenced this pull request Dec 10, 2018

Update smash tests for new schema

193cbbc

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added a commit to dralley/pulp that referenced this pull request Dec 10, 2018

Update smash tests for new schema

86cbc1e

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added a commit to dralley/pulp that referenced this pull request Dec 10, 2018

Update smash tests for new schema

5e7e770

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added a commit to dralley/pulp that referenced this pull request Dec 10, 2018

Update smash tests for new schema

a5b8c6f

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added a commit to dralley/pulp that referenced this pull request Dec 10, 2018

Update smash tests for new schema

298de2e

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added a commit to dralley/pulp that referenced this pull request Dec 10, 2018

Update smash tests for new schema

3aad46b

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley force-pushed the api-changes branch from 7a490d7 to 404bf74 Compare December 10, 2018 20:01

dralley added a commit to dralley/pulp that referenced this pull request Dec 10, 2018

Update smash tests for new schema

28d6ae0

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added a commit to dralley/pulp that referenced this pull request Dec 10, 2018

Update smash tests for new schema

7163485

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added a commit to dralley/pulp that referenced this pull request Dec 10, 2018

Update smash tests for new schema

0e41325

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley force-pushed the api-changes branch from 404bf74 to a077b6d Compare December 11, 2018 21:03

dralley added a commit to dralley/pulp that referenced this pull request Dec 11, 2018

Update smash tests for new schema

d0a7e2a

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley force-pushed the api-changes branch from a077b6d to e2bad07 Compare December 12, 2018 15:45

dralley added a commit to dralley/pulp that referenced this pull request Dec 12, 2018

Update smash tests for new schema

c1fbf56

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added a commit to dralley/pulp that referenced this pull request Dec 13, 2018

Update smash tests for new schema

4c36607

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added a commit to dralley/pulp that referenced this pull request Dec 13, 2018

Update smash tests for new schema

96f2479

Required PR: pulp/pulp_file#133 Required PR: pulp/pulp-smash#1154 [noissue]

dralley added a commit to dralley/pulp that referenced this pull request Dec 14, 2018

Update smash tests for new schema

df92015

Required PR: pulp/pulp-smash#1154 Required PR: pulp/pulp_file#133 [noissue]

dralley force-pushed the api-changes branch 3 times, most recently from c3e404e to 60a2891 Compare December 14, 2018 20:45

Adapting pulp_smash tests to new schema

ec56304

Required PR: pulp/pulp#3774 Required PR: pulp/pulp-smash#1154 [noissue]

dralley force-pushed the api-changes branch 2 times, most recently from 4286014 to 1634ba5 Compare December 17, 2018 17:08

dkliban approved these changes Dec 17, 2018

View reviewed changes

Use content summaries where possible.

575422f

Shorter and probably more efficient than fetching all the content data. [noissue]

dralley force-pushed the api-changes branch from 1634ba5 to 575422f Compare December 17, 2018 17:51

dralley added a commit to dralley/pulp that referenced this pull request Dec 17, 2018

Use content summaries where possible.

5aba336

Shorter and probably more efficient than fetching all the content data. Required PR: pulp/pulp-smash#1154 Required PR: pulp/pulp_file#133 [noissue]

dralley force-pushed the api-changes branch 4 times, most recently from ff29f65 to d5f43fb Compare December 17, 2018 19:45

dralley mentioned this pull request Dec 17, 2018

API Changes pulp/pulp-smash#1154

Merged

nixocio reviewed Dec 17, 2018

View reviewed changes

Add back support for Required PR with Pulp Smash

2286025

Required PR: pulp/pulp#3774 Required PR: pulp/pulp-smash#1154 [noissue]

dralley force-pushed the api-changes branch from d5f43fb to 2286025 Compare December 17, 2018 21:27

dkliban merged commit 2488131 into pulp:master Dec 17, 2018

dralley deleted the api-changes branch December 17, 2018 22:50

API Changes #133

API Changes #133

dralley commented Nov 27, 2018

dralley Nov 27, 2018 •

edited

nixocio Dec 4, 2018

dralley Dec 4, 2018

rochacbruno Dec 4, 2018

rochacbruno Dec 4, 2018

dralley Dec 4, 2018 •

edited

dralley Dec 4, 2018

dralley Dec 4, 2018

daviddavis Nov 27, 2018

dralley Nov 27, 2018

dralley commented Dec 4, 2018

dralley commented Dec 4, 2018

dkliban left a comment

dralley commented Dec 17, 2018

nixocio Dec 17, 2018

nixocio Dec 17, 2018

API Changes #133

API Changes #133

Conversation

dralley commented Nov 27, 2018

dralley Nov 27, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dralley Dec 4, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dralley commented Dec 4, 2018

dralley commented Dec 4, 2018

dkliban left a comment

Choose a reason for hiding this comment

dralley commented Dec 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dralley Nov 27, 2018 •

edited

dralley Dec 4, 2018 •

edited