Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/440
This is awesome. Thanks @takezoe !
---
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/438
LGTM. Merging. Thanks @takezoe !
---
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/436
LGTM. Merging. Thanks @marevol !
---
Github user dszeto commented on a diff in the pull request:
https://github.com/apache/incubator-predictionio/pull/439#discussion_r143087035
--- Diff: docs/manual/source/install/install-sourcecode.html.md.erb ---
@@ -28,15 +28,20 @@ Download Apache PredictionIO (incubating
Github user dszeto commented on a diff in the pull request:
https://github.com/apache/incubator-predictionio/pull/439#discussion_r143086982
--- Diff: docs/manual/source/install/install-sourcecode.html.md.erb ---
@@ -28,15 +28,20 @@ Download Apache PredictionIO (incubating
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/436
Hey @marevol , any idea with the failing test? I'm wondering if the new
Guava dependency is not working fine with older Hadoop/Spark.
---
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-sdk-ruby/pull/22
Thanks for fixing this, @anothermh ! Sorry that the contribution guide is
outdated. I will update it. Meanwhile, please follow
https://github.com/apache/incubator-predictionio
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-sdk-ruby/pull/21
Thanks for the fix @shouya ! Sorry that it took a bit to merge.
---
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/434
Thanks for catching!
---
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/433
Merging. Thanks @aayush142128 !
---
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/431
Verified that GitHub is forwarding repos from the previous user to the new
user. Merging. Thanks @thomasste !
---
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/367
I am convinced that this PR does not decrease the security of generating
random app access keys. I will merge if there is no objection.
---
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/427
Verified working with Python 2. Thanks!
---
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/427
@marevol Looks like Python 3 is also a hard requirement? This does not work
when I run `pyspark` with Python 2.
---
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/430
@mars may want to test this.
---
Github user dszeto commented on a diff in the pull request:
https://github.com/apache/incubator-predictionio/pull/424#discussion_r134588503
--- Diff:
data/src/main/scala/org/apache/predictionio/data/store/Common.scala ---
@@ -19,35 +19,46 @@
package
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/387
You will need to use `docker` commands to either read volumes that were
attached to running containers. The test is designed to return non-zero exit
code if any tests fail, so if you
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/418
LGTM. Thanks @mars !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/405
@takezoe It's done by our Jenkins project on ASF:
https://builds.apache.org/job/PredictionIO-build-site/69/console
Looks like the site built correctly but the publish step
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/417
Hey, what do you think about putting Release Cadence under the Getting
Involved section? We should probably clean up the Resources section because it
sounds pretty generic
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/412
Created https://issues.apache.org/jira/browse/PIO-110 and
https://issues.apache.org/jira/browse/PIO-111 as follow-ups. Thanks @mars for
the feature and @takezoe for the feedback
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/406
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/387
These reports are generated by `XMLTestRunner` in
`tests/pio_tests/tests.py`, which are generated inside the container.
---
If your project is set up for it, you can reply
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/401
LGTM. Thanks @chanlee514 @mars !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/405
@takezoe @shimamoto Sorry for the late reply. I took a look and I think the
current approach is good. Thanks!
---
If your project is set up for it, you can reply to this email
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/407
LGTM. Thanks @takezoe !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/355
Sorry for the late reply. LGTM and merging. Thanks @lucasbm88 !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/387
Should be
```
tests/run_docker.sh $METADATA_REP $EVENTDATA_REP $MODELDATA_REP \
"python3 /PredictionIO/tests/pio_tests/tests.py"
```
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/387
@liningalex Let us know how we can help. This is a great PR and we would
like to get it in.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/412
LGTM. Great work @mars !
Agree with @takezoe 's comments. I think there are a few follow up tasks we
should do.
1. Refactor common code that exists in both
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/404
LGTM. I checked the dependency tree and looks like this is the easiest way
out. Many transitive dependencies rely on `slf4j-log4j12`.
---
If your project is set up for it, you can
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/387
We don't have any documentation (which we should add later) on how that's
done, but basically, it would be adding a new combination in the test matrix
(https://github.com/apache
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/400
LGTM. Please feel free to merge once it's reviewed by one of us (in this
case, @takezoe).
Thanks @shimamoto !
---
If your project is set up for it, you can reply
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/397
@takezoe Excellent work. Thanks! ð
I think the only concern left is to make sure whenever we add a new
dependency that does not have clear indication of license
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/391
Sorry for the late reply. Agree we should start properly deprecating it.
Regarding the inconsistency, I think we should correct `0.98.6 in
https://predictionio.incubator.apache.org
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/393
LGTM. Thanks @mars !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/394
@mars let's file a different JIRA for exposing engine server configuration
in a generic way. Thanks!
---
If your project is set up for it, you can reply to this email and have your
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/391
@takezoe LGTM. Mind also updating the documentation in this PR to reflect
the new minimum requirement (http://predictionio.incubator.apache.org/install/)?
---
If your project is set
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/387
@liningalex Is the main motivation for a separate `OJDBC` set of classes
due to Oracle requiring different column types?
---
If your project is set up for it, you can reply
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/390
@duksis asfgit closes it automatically when your change is merged to
develop. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user dszeto commented on a diff in the pull request:
https://github.com/apache/incubator-predictionio/pull/385#discussion_r120960881
--- Diff: bin/compute-classpath.sh ---
@@ -17,36 +17,35 @@
# limitations under the License.
#
-SCALA_VERSION=2.10
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/383
Merged. Thanks @takezoe !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/387
@liningalex could you also please open a ticket on our Apache JIRA to help
with tracking? Thanks!
I will conduct a more thorough review ASAP.
---
If your project is set up
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/387
@takezoe A quick search reveals
https://hub.docker.com/r/wnameless/oracle-xe-11g/, but this limits the
guarantee to whatever Oracle XE 11g supports. @liningalex would you mind taking
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/368
Hey @steven-haddix , sorry for the late response and thanks for the PR. A
couple comments about this PR.
The Linux manual installation page is supposed to be used with binary
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-sdk-ruby/pull/20
LGTM. Thanks @kahirul !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-sdk-ruby/pull/20
LGTM. Thanks @kahirul !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-sdk-ruby/pull/19
LGTM. Thanks @ZhKostev !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/383
We should perhaps ping them again so we don't have to be stuck with Travis:
https://issues.apache.org/jira/browse/INFRA-13543
---
If your project is set up for it, you can reply
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/382
Hey @emergentorder, do you have a chance to take a look at this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/377
To obtain committership, one would contribute and earn a nomination from a
PMC member. Once the PMC voted favorably, the nominee will gain committership
(the ability to push code
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/issues/380
Hi, please redirect usage questions to
u...@predictionio.incubator.apache.org, and development question to
dev@predictionio.incubator.apache.org for more exposure to the rest
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-template-attribute-based-classifier/pull/10
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-template-attribute-based-classifier/pull/9
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does
Github user dszeto commented on a diff in the pull request:
https://github.com/apache/incubator-predictionio-template-attribute-based-classifier/pull/5#discussion_r114821537
--- Diff: src/main/scala/Engine.scala ---
@@ -3,11 +3,13 @@ package org.template.classification
import
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-template-ecom-recommender/pull/12
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-template-ecom-recommender/pull/4
Hi @goliasz , is this still work in progress or ready to merge?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-template-skeleton/pull/6
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-template-recommender/pull/15
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-template-recommender/pull/4
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio-template-recommender/pull/12
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/371
@marevol Would you like to add some integration test for this?
https://github.com/jubos/fake-s3 or https://github.com/atlassian/localstack
might be useful for mocking S3 locally
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/378
Test failure irrelevant to the change. Filed a ticket to track fixing
tests: https://issues.apache.org/jira/browse/PIO-64
---
If your project is set up for it, you can reply
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/376
@takezoe Guidelines are here:
http://predictionio.incubator.apache.org/community/contribute-documentation/
tl;dr
- Use the livedoc branch if you want to update
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/377
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/374
LGTM. I'd like to suggest us to get into the habit of filing JIRA tickets
for all changes. It will make the release notes writer easier to figure out
what's been changed
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/373
LGTM. By the way, are you working with ES 5.2/5.3? @mars bumped into issues
with ES 5.1.
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user dszeto commented on a diff in the pull request:
https://github.com/apache/incubator-predictionio/pull/371#discussion_r113326594
--- Diff: storage/s3/build.sbt ---
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/370
LGTM. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/370
Thanks @marevol ! Do you know if the REST client allows configuration by
file? For the old ES1 client, one could supply an `elasticsearch.yml` file for
the client to read in the rest
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/issues/363
Hi, please redirect usage questions to
u...@predictionio.incubator.apache.org, and development question to
dev@predictionio.incubator.apache.org for more exposure to the rest
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/359
Hey @marevol , running into this:
```
+ sbt/sbt assembly/rpm:packageBin
[info] Loading global plugins from /Users/dszeto/.sbt/0.13/plugins
[info] Loading project
Github user dszeto commented on a diff in the pull request:
https://github.com/apache/incubator-predictionio/pull/365#discussion_r108459590
--- Diff: tools/build.sbt ---
@@ -27,8 +27,6 @@ libraryDependencies ++= Seq(
"io.spray" %% "spray-tes
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/364
I would still like to merge some of your changes if you are okay with that.
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/362
Thanks for spotting this! The latest unit tests are actually failing but
exited with a 0 code. Debugging.
---
If your project is set up for it, you can reply to this email and have
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/362
@takezoe How did you trigger the compilation error?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/359
This is nice work! Would it be easier to wrap it with a script like what we
are doing with make-distribution.sh? Also we are looking to release 0.11 soon
once PIO-30 finished testing
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/361
Awesome. Thanks @linamy85 !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user dszeto commented on a diff in the pull request:
https://github.com/apache/incubator-predictionio/pull/357#discussion_r105085253
--- Diff:
tools/src/main/scala/org/apache/predictionio/tools/console/Console.scala ---
@@ -734,7 +734,7 @@ object Console extends Logging
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/355
@lucasbm88 PIO-49 has been merged. Please take a look. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/334
Thanks @anandaverma and @bansarishah !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/339
Thanks @laertispappas !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/347
Thanks for your reminder @marevol . I was planning to do so after fixing
integration tests.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/352
So basically this will build PIO with the same behavior as before, and pack
the ES5 support as an extra JAR during `make-distribution.sh` in `lib/extra`.
In order to use ES5, one
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/issues/351
Hi, please redirect usage questions to
u...@predictionio.incubator.apache.org, and development question to
dev@predictionio.incubator.apache.org for more exposure to the rest
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/352
Hey @marevol are you working on fixing the integration tests? I can help if
you are not already doing so.
---
If your project is set up for it, you can reply to this email and have
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/352
Hi @marevol , I have finished working on PIO-53, which allows unit and
integration tests be updated per commit and removed reliance on a cached Docker
image on Docker Hub. You might
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/352
Adding @haginot to this thread.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/353
@vshwnth2 Are you still maintaining your template? Are you okay with this
change? Will merge this if there isn't a reply in a couple days.
---
If your project is set up for it, you
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/349
Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/336
What I meant was the `lib` folder in the binary distribution tarball after
doing `make-distribution.sh`. There isn't really a convention but I have seen
many Apache binary
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/336
Thanks @marevol ! @pferrel do you want to push your update on the start and
stop scripts?
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/336
Yes. I've created and pushed a branch (feature/es5). @pferrel is verifying
with universal recommender.
On Mon, Feb 13, 2017 at 12:37 AM Shinsuke Sugaya <notific
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/336
@haginot @marevol @pferrel After putting more thought to this, we should
probably rename `elasticsearch1` back to `elasticsearch`, and have ES5 under
`elasticsearch5` package name
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/issues/341
Hi, please redirect usage questions to
u...@predictionio.incubator.apache.org for more exposure to the rest of the
community. Subscription instructions can be found at
http
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/336
I'm going to do some more manual testing. I'll report back and let's merge
this if things look well.
---
If your project is set up for it, you can reply to this email and have your
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/336
If there's no objection, I suggest merging this first and do the other
suggested optimizations in a separate JIRA/PR later.
---
If your project is set up for it, you can reply
Github user dszeto commented on a diff in the pull request:
https://github.com/apache/incubator-predictionio/pull/336#discussion_r96759997
--- Diff:
data/src/main/scala/org/apache/predictionio/data/storage/elasticsearch/ESAccessKeys.scala
---
@@ -15,44 +15,45
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/336
Reviewing the code right now. It would be nice if we could include a
migration guide in the docs if possible.
---
If your project is set up for it, you can reply to this email
Github user dszeto commented on the issue:
https://github.com/apache/incubator-predictionio/pull/336
This is really great. Thanks a lot @haginot and @marevol !
Officially ES 1.7 and before are deprecated. I will start a thread in user@
and dev@ to gauge if we need to maintain
1 - 100 of 145 matches
Mail list logo
