Re: Review Request 45177: Prototype of setting DiscoveryInfo.

2016-03-25 Thread Stephan Erb 


> On March 25, 2016, 9:30 p.m., Stephan Erb wrote:
> > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java, line 
> > 237
> > 
> >
> > That can lead to non-DNS compatible names. I guess there is nothering 
> > that we can really do about it. It's just something consumers have to be 
> > aware of.

Looks like we don't have to worry: 
https://github.com/apache/aurora/blob/26efe5517fc0cb471101fdcb072e5dbf5d20bc56/api/src/main/thrift/org/apache/aurora/gen/api.thrift#L426


- Stephan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45177/#review125456
---


On March 22, 2016, 9:10 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45177/
> ---
> 
> (Updated March 22, 2016, 9:10 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Prototype of setting DiscoveryInfo.
> 
> Disclaimer: this just shows how easy it is to pipe this information, not 
> seeking to commit the code as is.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 20cbd419dcb988f2c7ba72c8acbe6fee90d02248 
> 
> Diff: https://reviews.apache.org/r/45177/diff/
> 
> 
> Testing
> ---
> 
> Use the vagrant environment to start the example in http_example.aurora, and 
> observe the following in master's state.json:
> 
> ```
> curl 192.168.33.7:5050/state | jq . | less
> 
> "tasks": [
> {
> "discovery": {
> "environment": "test",
> "name": "vagrant.test.http_example.1",
> "ports": {
> "ports": [
> {
> "name": "http",
> "number": 31648
> }
> ]
> },
> "visibility": "CLUSTER"
> },
> "executor_id": 
> "thermos-vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "framework_id": "770e194c-6366-41f2-8a2d-e2c616aa9490-",
> "id": "vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "name": "vagrant/test/http_example",
> "resources": {
> "cpus": 0.4,
> "disk": 64,
> "mem": 32,
> "ports": "[31648-31648]"
> },
> ...
> 
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 45177: Prototype of setting DiscoveryInfo.

2016-03-25 Thread Stephan Erb 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45177/#review125456
---




src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 217)


Sounds OK as hard coded value for now.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 218)


That can lead to non-DNS compatible names. I guess there is nothering that 
we can really do about it. It's just something consumers have to be aware of.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 220)


That sounds like a sane choice, but we could also leave it out for now 
until there is a proper usecase for it.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 221)


I guess this refers to the API version of the task? The Mesos documentation 
is not clear here. I'd vote to keep it out for now.



src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java (line 224)


We could probably do this via the `Announcer` struct. Unfortunately, the 
documentation is rather unclear if we should pass L4 or L7 protocol names: 
https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L1799

However, looks like Marathon is saying this should by only `TCP` or `UDP`: 
https://github.com/mesosphere/marathon/blob/master/src/main/scala/mesosphere/marathon/state/DiscoveryInfo.scala#L25
 Hardcoding this value to `TCP` should therefore probably work for us for now, 
right?


- Stephan Erb


On March 22, 2016, 9:10 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45177/
> ---
> 
> (Updated March 22, 2016, 9:10 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Prototype of setting DiscoveryInfo.
> 
> Disclaimer: this just shows how easy it is to pipe this information, not 
> seeking to commit the code as is.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 20cbd419dcb988f2c7ba72c8acbe6fee90d02248 
> 
> Diff: https://reviews.apache.org/r/45177/diff/
> 
> 
> Testing
> ---
> 
> Use the vagrant environment to start the example in http_example.aurora, and 
> observe the following in master's state.json:
> 
> ```
> curl 192.168.33.7:5050/state | jq . | less
> 
> "tasks": [
> {
> "discovery": {
> "environment": "test",
> "name": "vagrant.test.http_example.1",
> "ports": {
> "ports": [
> {
> "name": "http",
> "number": 31648
> }
> ]
> },
> "visibility": "CLUSTER"
> },
> "executor_id": 
> "thermos-vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "framework_id": "770e194c-6366-41f2-8a2d-e2c616aa9490-",
> "id": "vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "name": "vagrant/test/http_example",
> "resources": {
> "cpus": 0.4,
> "disk": 64,
> "mem": 32,
> "ports": "[31648-31648]"
> },
> ...
> 
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 45177: Prototype of setting DiscoveryInfo.

2016-03-22 Thread Aurora ReviewBot 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/45177/#review124885
---



Master (b5c9e1b) is red with this patch.
  ./build-support/jenkins/build.sh

:compileJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java:74:
 Note: Wrote forwarder 
org.apache.aurora.scheduler.storage.log.WriteAheadStorageForwarder
@Forward({
^
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.2
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor

:generateBuildProperties
:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJavaNote: 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java
 uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:20:
 error: 'org.apache.aurora.GuavaUtils' should be separated from previous 
imports.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:31:
 error: Using the '.*' form of import should be avoided - 
org.apache.aurora.scheduler.storage.entities.*.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:33:
 error: Using the '.*' form of import should be avoided - 
org.apache.mesos.Protos.*.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:37:
 error: Wrong order for 'javax.inject.Inject' import. Order should be: java, 
javax, scala, com, net, org. Each group should be separated by a single 
blank line.
[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java:38:
 error: Wrong order for 'java.util.List' import. Order should be: java, javax, 
scala, com, net, org. Each group should be separated by a single blank 
line.
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.html

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

BUILD FAILED

Total time: 1 mins 44.437 secs


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On March 22, 2016, 8:10 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45177/
> ---
> 
> (Updated March 22, 2016, 8:10 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Prototype of setting DiscoveryInfo.
> 
> Disclaimer: this just shows how easy it is to pipe this information, not 
> seeking to commit the code as is.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 20cbd419dcb988f2c7ba72c8acbe6fee90d02248 
> 
> Diff: https://reviews.apache.org/r/45177/diff/
> 
> 
> Testing
> ---
> 
> Use the vagrant environment to start the example in http_example.aurora, and 
> observe the following in master's state.json:
> 
> ```
> curl 192.168.33.7:5050/state | jq . | less
> 
> "tasks": [
> {
> "discovery": {
> "environment": "test",
> "name": "vagrant.test.http_example.1",
> "ports": {
> "ports": [
> {
> "name": "http",
> "number": 31648
> }
> ]
> },
> "visibility": "CLUSTER"
> },
> "executor_id": 
> "thermos-vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "framework_id": "770e194c-6366-41f2-8a2d-e2c616aa9490-",
> "id": "vagrant-test-http_example-1-69c3908a-e8cd-44a7-a824-dd87ef5920f0",
> "name": "vagrant/test/http_example",
> "resources": {
> "cpus": 0.4,
> "disk": 64,
> "mem": 32,
> "ports": "[31648-31648]"
> },
> ...
> 
> ```
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>