Adds the content app to pulpcore.content #3779

bmbouter · 2018-11-30T21:55:54Z

The Settings can be a lot simpler now that the architecture no longer
required redirection. It does need to account for the content being
served on a different HOST. This PR handles that.

Since the settings are now single values, they are much easier to
override using Dynaconf, which is great.

The content app can be removed now that the streamer code is handling it
all.

This PR is a prototype and has no docs. Use Pulp as normal , but run the
streamer from github.com/bmbouter/pulp_streamer/ and run the streamer
with:

gunicorn pulpcore.streamer:server --bind localhost:8080 --worker-class aiohttp.GunicornWebWorker -w 2

This PR needs pulp-smash changes from pulp_file so it requires the PR
below:

Required PR: pulp/pulp_file#140

https://pulp.plan.io/issues/4239
closes #4239

pep8speaks · 2018-11-30T21:55:56Z

Hello @bmbouter! Thanks for updating the PR.

There are no PEP8 issues in the file pulpcore/app/models/repository.py !
There are no PEP8 issues in the file pulpcore/app/serializers/fields.py !
There are no PEP8 issues in the file pulpcore/app/settings.py !
There are no PEP8 issues in the file pulpcore/app/urls.py !
There are no PEP8 issues in the file pulpcore/app/views/__init__.py !
There are no PEP8 issues in the file pulpcore/content/__init__.py !
There are no PEP8 issues in the file pulpcore/content/handler.py !

Comment last updated on December 12, 2018 at 18:04 Hours UTC

This is designed to be used with this PR against core: pulp/pulp#3779

.travis/install.sh

codecov · 2018-12-12T20:47:36Z

Codecov Report

Merging #3779 into master will decrease coverage by 0.64%.
The diff coverage is 3.2%.

@@            Coverage Diff             @@
##           master    #3779      +/-   ##
==========================================
- Coverage   54.02%   53.37%   -0.65%     
==========================================
  Files          63       64       +1     
  Lines        2758     2739      -19     
==========================================
- Hits         1490     1462      -28     
- Misses       1268     1277       +9

Impacted Files	Coverage Δ
pulpcore/app/views/__init__.py	`100% <ø> (ø)`	⬆️
pulpcore/app/serializers/fields.py	`44.64% <0%> (ø)`	⬆️
pulpcore/content/handler.py	`0% <0%> (ø)`
pulpcore/content/__init__.py	`0% <0%> (ø)`
pulpcore/app/urls.py	`91.93% <100%> (ø)`	⬆️
pulpcore/app/models/repository.py	`55.17% <100%> (ø)`	⬆️
pulpcore/app/settings.py	`97.72% <100%> (+0.05%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9713e27...8219efd. Read the comment docs.

bmbouter · 2018-12-13T15:50:19Z

FYI since this PR removes the apache and nginx file serving integration I'm making another issue to bring that back. I'll post a link to that other issue here today. We can prioritize that together and I'm happy to bring it back immediately in another PR if that is what we should do. I'll link to the issue when I make it.

jortel · 2018-12-13T16:29:12Z

LGTM
Please squash before merging. Looks like @dkliban has requested changes so perhaps he should approve when satisfied.

The Settings can be a lot simpler now that the architecture no longer required redirection. It does need to account for the content being served on a different HOST. This PR handles that. Since the settings are now single values, they are much easier to override using Dynaconf, which is great. The content app can be removed now that the streamer code is handling it all. This PR is a prototype and has no docs. Use Pulp as normal , but run the streamer from github.com/bmbouter/pulp_streamer/ and run the streamer with: `gunicorn pulpcore.streamer:server --bind localhost:8080 --worker-class aiohttp.GunicornWebWorker -w 2` This PR needs pulp-smash changes from pulp_file so it requires the PR below: Required PR: pulp/pulp_file#140 https://pulp.plan.io/issues/4239 closes #4239 https://pulp.plan.io/issues/3698 closes pulp#3698 https://pulp.plan.io/issues/3699 closes pulp#3699 https://pulp.plan.io/issues/4181 closes #4181 https://pulp.plan.io/issues/4243 closes #4243

dkliban

Thank you!

dralley · 2018-12-14T04:51:30Z

@bmbouter The Travis config changes in this PR need to be copied over to plugins and the plugin template, since without the content app or the streamer, the "download content" tests fail.

dralley · 2018-12-14T04:54:53Z

Also our docs on how to run Pulp, both the hosted docs and the plugin docs e.g. https://github.com/pulp/pulp_file/#run-services. Since PRs need to be made against the plugins anyway it would be good to fix that at the same time.

bmbouter · 2018-12-14T13:55:30Z

@dralley I'm working on fixing Travis for the plugins and after that I'm fixing the docs. Hopefully all will get done today.

This updates Travis to continue to pass given the breaking change merged to core with this PR: pulp/pulp#3779

bmbouter added Work In Progress 3.0 labels Nov 30, 2018

bmbouter added a commit to bmbouter/pulpcore-content that referenced this pull request Nov 30, 2018

Serves content according to Distribution.base_url

fb5f1c7

This is designed to be used with this PR against core: pulp/pulp#3779

bmbouter force-pushed the replace-content-app-with-streamer branch from dc22dc6 to 5aeba1f Compare November 30, 2018 22:00

bmbouter added a commit to bmbouter/pulpcore-content that referenced this pull request Dec 3, 2018

Serves content according to Distribution.base_url

ceaee0b

This is designed to be used with this PR against core: pulp/pulp#3779

bmbouter force-pushed the replace-content-app-with-streamer branch 7 times, most recently from 1820944 to 40c5390 Compare December 11, 2018 16:50

dkliban reviewed Dec 11, 2018

View reviewed changes

.travis/install.sh Outdated Show resolved Hide resolved

bmbouter force-pushed the replace-content-app-with-streamer branch 11 times, most recently from 5882498 to c06f6e7 Compare December 12, 2018 19:45

bmbouter changed the title ~~Removes content app and reworks settings~~ Adds the content app to pulpcore.content Dec 12, 2018

bmbouter force-pushed the replace-content-app-with-streamer branch 3 times, most recently from ccce3c8 to bea84e2 Compare December 12, 2018 20:39

bmbouter added Feature lazy Issue and removed Work In Progress labels Dec 12, 2018

bmbouter force-pushed the replace-content-app-with-streamer branch from bea84e2 to b5c8d37 Compare December 12, 2018 22:09

bmbouter force-pushed the replace-content-app-with-streamer branch from b5c8d37 to 8219efd Compare December 13, 2018 17:19

dkliban approved these changes Dec 13, 2018

View reviewed changes

bmbouter merged commit a3aaa5e into pulp:master Dec 13, 2018

bmbouter deleted the replace-content-app-with-streamer branch December 13, 2018 17:59

bmbouter pushed a commit to bmbouter/pulp_gem that referenced this pull request Dec 14, 2018

Updates for the new content app on 'master'

fea6f56

This updates Travis to continue to pass given the breaking change merged to core with this PR: pulp/pulp#3779

bmbouter mentioned this pull request Dec 14, 2018

Updates for the new content app on 'master' pulp/pulp_gem#14

Closed

bmbouter pushed a commit to bmbouter/pulp_cookbook that referenced this pull request Dec 14, 2018

Updates for the new content app on 'master'

613662c

This updates Travis to continue to pass given the breaking change merged to core with this PR: pulp/pulp#3779

bmbouter mentioned this pull request Dec 14, 2018

Updates for the new content app on 'master' pulp/pulp_cookbook#10

Merged

bmbouter pushed a commit to bmbouter/pulp_cookbook that referenced this pull request Dec 14, 2018

Updates for the new content app on 'master'

d29e0d1

This updates Travis to continue to pass given the breaking change merged to core with this PR: pulp/pulp#3779

gmbnomis pushed a commit to pulp/pulp_cookbook that referenced this pull request Dec 15, 2018

Updates for the new content app on 'master'

436f6b3

This updates Travis to continue to pass given the breaking change merged to core with this PR: pulp/pulp#3779

mdellweg mentioned this pull request Dec 17, 2018

Require 'requires' in PR regex pulp/pulp_gem#15

Merged

Adds the content app to pulpcore.content #3779

Adds the content app to pulpcore.content #3779

bmbouter commented Nov 30, 2018 •

edited

pep8speaks commented Nov 30, 2018 •

edited

codecov bot commented Dec 12, 2018 •

edited

bmbouter commented Dec 13, 2018

jortel commented Dec 13, 2018

dkliban left a comment

dralley commented Dec 14, 2018

dralley commented Dec 14, 2018 •

edited

bmbouter commented Dec 14, 2018

Adds the content app to pulpcore.content #3779

Adds the content app to pulpcore.content #3779

Conversation

bmbouter commented Nov 30, 2018 • edited

pep8speaks commented Nov 30, 2018 • edited

Comment last updated on December 12, 2018 at 18:04 Hours UTC

codecov bot commented Dec 12, 2018 • edited

Codecov Report

bmbouter commented Dec 13, 2018

jortel commented Dec 13, 2018

dkliban left a comment

Choose a reason for hiding this comment

dralley commented Dec 14, 2018

dralley commented Dec 14, 2018 • edited

bmbouter commented Dec 14, 2018

bmbouter commented Nov 30, 2018 •

edited

pep8speaks commented Nov 30, 2018 •

edited

codecov bot commented Dec 12, 2018 •

edited

dralley commented Dec 14, 2018 •

edited