Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

[WIP] Incorporate the official templates in core repository #500

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

takezoe
Copy link
Member

@takezoe takezoe commented Nov 19, 2018

I'm considering about incorporate the official templates with main repository to reduce maintenance costs of them. My idea is:

  • Check versions of PredictionIO/Scala/Spark/etc by TravisCI using a shell script and if these versions are not same among PredictionIO and templates, test will fail.
  • Include the official templates and examples in the distribution which is made by make-distribution.sh.
  • Documentations which refer these templates or examples will be fixed to be started from:
    $ cd $PIO_HOME/templates/template-scala-parallel-recommendation
    $ pio build
    ...
    
    instead of:
    $ git clone https://github.com/apache/predictionio-template-recommender.git
    $ cd predictionio-template-recommender
    $ pio build
    ...
    

Currently, for every release, we have to update versions of PredictionIO/Scala/Spark/etc, then create tag for each template repository by hand. If the official templates are in the same repository with PredictionIO itself, we will be able to finish this boring operation at once. Also, it will ensure that all official templates is synchronized with the latest PredictionIO version.

@Wei-1
Copy link
Member

Wei-1 commented Nov 19, 2018

I think it causes some confusions when new users try to learn PIO and found that they will need to clone the template from another repository.
So it is definitely better for us to put it together with the main repo.
Since we already have those examples in the main repo, I think it makes a lot of sense.

One question that I will bring up is:
Are we still going to use the package org.example.recommendation?
or maybe we could change it to org.apache.predictionio.template?

@takezoe
Copy link
Member Author

takezoe commented Nov 19, 2018

@Wei-1

Are we still going to use the package org.example.recommendation?
or maybe we could change it to org.apache.predictionio.template?

We should locate templates under org.apache.predictionio package if we will distribute templates with distribution. Thanks!

@@ -0,0 +1,202 @@

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the license file necessary for each template after they are incorporated into this repository?

Please refer to
https://predictionio.apache.org/templates/recommendation/quickstart/.

## Versions
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will have to maintain README of each template after incorporating, I think it should be synchronized with PredictionIO release, but text-classifier template has inconsistent versioning with PredictionIO version. How should we handle that?

In addition, text-classifier template contains large data in its repository, I want to separate that data from the repository. Is there any suitable place to put them?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the test you added to check_templates.sh is a good idea.
People will know that they have to change the corresponding template version while modifying PIO's version.
As for the data, maybe we add another non-test branch?

@@ -43,4 +43,6 @@ sbt/sbt dataJdbc/compile test storage/test \
-Delasticsearch.version=$PIO_ELASTICSEARCH_VERSION \
-Dhbase.version=$PIO_HBASE_VERSION

./tests/check_templates.sh
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add incorporated templates here to check by TravisCI.

@@ -0,0 +1,57 @@
#!/bin/bash
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This script is run after unit tests on TravisCI to check version consistency and test templates.

@shimamoto
Copy link
Member

@takezoe , sorry for the late response.

Basically, I agree with incorporating them into the main repository. I'm concerned about the following before this PR.

  • What templates we incorporate into the main repository
    We might as well discuss whether we continue to maintain all of the current official templates.

  • Namespace
    Though the official templates are maintained by committers, they are NOT as part of main modules but as part of examples. I think they should be under examples folder and have org.apache.predictionio.examples package.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants