[Pulp-dev] Plugin Writer's Coding Workshop Feedback

Tue Feb 13 15:50:26 UTC 2018

Thanks for providing this feedback, Brian.!  Good stuff.

On 02/12/2018 03:57 PM, Brian Bouterse wrote:
> At the Foreman Construction day [0] last Wednesday, we had our first 
> code focused plugin writer's workshop. About 6 people were actively 
> engaged as we talked through the plugin API, example code, and then 
> tried to install Pulp3. All of this happened over about 4-5 hours. In 
> contrast to the devconf workshop which was planning focused, this was 
> a "let's look at and write some code together" workshop. Two attendees 
> came to both, and they got all the way to calling their own sync code.
>
> We got a lot of feedback, which I will try to group into some areas. 
> (my feedback in parens)
>
> [installation issues]
> - the pypi install commands are missing the migrations and they 
> produce broken installations
> - the vagrantcloud boxes couldn't have a plugin installed on them :(
> - the dev environments worked great but we didn't recommend them until 
> we realized all of these other methods were broken
> - we assume the user 'pulp' in a lot of places, e.g. systemd file, 
> ansible, etc
> - assumptions about Fedora both in ansible, but also the copy/paste 
> commands
> - some users who copied and pasted commands didn't realize they 
> weren't for their OS
>
> [desire for simpler things]
> - there is a strong desire to use sqlite as the default db not postgresql

Very interesting.  Can you elaborate about why?

> - desire to not install a message bus. (I think this is unavoidable)
>
> [need examples]
> - pulp_file is our example, but it's laid out into different functions 
> and classes. People were confused by this because they thought the 
> classes and function names are meaningful when they aren't. For 
> example we were asked "what is a synchronizer" 
> https://github.com/pulp/pulp_file/blob/master/pulp_file/app/tasks.py#L139

The Synchronizer used to be the FileImporter and got renamed as part of 
early mitigation of the "circular import" problem.  I plan to do some 
final refactoring as soon as the plugin API stabilizes (really soon).  I 
suspect the Synchronizer class (at least the name), will go away.  That 
said, I'm a little puzzled as to what led to actual "confusion" about a 
class named Synchronizer that was used to synchronize a repository.  You 
also mentioned that some of the function names where somehow confusing - 
can you name them and why they were confusing?

> - pulp_file doesn't provide a good example because changesets do 
> everything for you. (The main pulp_file should be a simple, direct 
> example of the objects they have to save).

True, but It does provides a good example of how to use the ChangeSet.

> - people found pulp_example via github and thought "oh here is what I 
> needed to find!" only to base their code on outdated code (we need to 
> delete pulp_example)
> - a database picture would be helpful to show off the data layer 
> objects, foreign keys, and attributes.

Yes!  We really need to publish an ER diagram.  I'm overdue on an action 
item to produce one.

>
> [specific things]
> - 'id' on the inherited content unit conflicted with a content unit 
> which also uses 'id'.
> - qpid vs rabbitmq defaults confusion. The settings.yaml says we 
> default to qpid so they installed qpid, but really in settings.py it's 
> rabbitmq. (this is a 1 line fix)
>
>
> In terms of the installation challenges, we should consider 
> consolidating onto a single installation method of pip with 
> virtualenv. Of all the methods we offer [1] that is the one everyone 
> could use and no one minded. We could remove the other options from 
> the install page so that for for now (pre-GA) everyone is doing the 
> same, simple thing. I think we should consolidate our effort and not 
> focus on end-user installations as the main thing right now.**
>
> I also think we should do these things:
>
> * switch pulp to use sqlite3 by default. An ansible installer can both 
> install postgres and configure it, later.
> * rewrite pulp_file to be a really really simple example

The file-plugin is already a "really really simple example". Rewriting 
it not using the ChangeSet will significantly increase code line count 
and complexity.  As you know, the file-plugin supports managing 
FileContent like .img and .iso files.  The primary goal of the pulp-file 
project is to support real use cases.  Because it's the only plugin, it 
has taken on a secondary goal of being an example.  I'm opposed to 
increasing complexity in support of the secondary "example" goal at the 
expense of its primary goal.

The file-plugin currently provides a good example of how to use the 
ChangeSet.  I have no doubt that plugin writers want additional examples 
but I think that if we intend to continue to rely on "real" plugins as 
natural examples, we should identify a plugin on the roadmap that has 
made the design choice to be implemented without the ChangeSet and 
prioritize it.  Another choice could be to refactor the example plugin 
to support a broader range of examples and continue to maintain it.

> * delete pulp_example
>
> Please send ideas, questions, or any kind of feedback.
>
> [0]: http://cfgmgmtcamp.eu/fringe.html#foreman
> [1]: https://docs.pulpproject.org/en/3.0/nightly/installation/index.html
>
> ** I still see Ansible as the right cross-distro installer as we 
> approach the GA date. @ichimonji10 I  am still +1 on your proposal, I 
> think we just need to consolidate both dev and testing effort for now. 
> This is similar to the approach for the migration tool which we know 
> is really important but we aren't starting yet.
>
> -Brian
>
>
>
> _______________________________________________
> Pulp-dev mailing list
> Pulp-dev at redhat.com
> https://www.redhat.com/mailman/listinfo/pulp-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20180213/84e5c336/attachment.htm>