No problem. We non-English folks have complicated names :-)
> On Nov 4, 2014, at 12:51 PM, Ankur Chauhan <an...@malloc64.com> wrote:
>
> Hi,
>
> Sorry about that bernd! :s/ben/bernd/g
>
> -- ankur
> Sent from my iPhone
>
>> On Nov 4, 2014, at 3:13 AM, Bernd Mathiske <be...@mesosphere.io> wrote:
>>
>> Hi Ankur,
>>
>> I am Bernd, not Ben, but I’ll try to do my best :-)
>>
>> Your plan looks good to me and your patch for MESOS-1711 seems uncomplicated
>> enough to not cause me major problems no matter when it lands.
>>
>> Bernd
>>
>>> On Nov 4, 2014, at 11:44 AM, Ankur Chauhan <an...@malloc64.com> wrote:
>>>
>>> Hi ben,
>>>
>>> Thanks for the follow up. I think thats a perfectly fine game plan. I think
>>> I can break down things into smaller, more isolated chunks. But from the
>>> looks of MESOS-1316 the invocation code and the testing code seems to
>>> change a lot (in a good way), so to avoid a bunch of wasted cycles, I was
>>> going to hold off a little till it is committed and then go on to
>>> refactoring the transport parts. The json config change (MESOS-1248) is
>>> completely disconnected to this so I am not too worried about that one.
>>> The only place where some coordination may be required is the
>>> launcher/fetcher.cpp. It seems there are common changes happening between
>>> my change https://reviews.apache.org/r/27483/
>>> <https://reviews.apache.org/r/27483/><https://reviews.apache.org/r/27483/
>>> <https://reviews.apache.org/r/27483/>> and your
>>> https://reviews.apache.org/r/21316
>>> <https://reviews.apache.org/r/21316><https://reviews.apache.org/r/21316
>>> <https://reviews.apache.org/r/21316>> which would mean a little rework but
>>> it's totally within reasonable limits.
>>> I think if you get the MESOS-1316 committed and I get MESOS-1711 (
>>> https://reviews.apache.org/r/27483 <https://reviews.apache.org/r/27483>
>>> <https://reviews.apache.org/r/27483 <https://reviews.apache.org/r/27483>> )
>>> in, both of us would get unblocked in a way that we can get a way that lets
>>> us concurrently work on MESOS-336 and refactoring fetcher into more
>>> testable parts.
>>>
>>> Does this make sense to you or did I misunderstand some part?
>>>
>>> -- Ankur
>>>
>>>> On 4 Nov 2014, at 02:05, Bernd Mathiske <be...@mesosphere.io
>>>> <mailto:be...@mesosphere.io>> wrote:
>>>>
>>>> Typo: not -> note
>>>>
>>>>> On Nov 4, 2014, at 10:59 AM, Bernd Mathiske <be...@mesosphere.io
>>>>> <mailto:be...@mesosphere.io>> wrote:
>>>>>
>>>>> Hi Ankur,
>>>>>
>>>>> I like it, too. However, I cannot refrain from relaying (not my choice of
>>>>> word here) to you the advice to break your relatively large patch down
>>>>> into smaller parts. My patch for MESOS-336 certainly was, as I know now.
>>>>> My plan is to get MESOS-1316 (which is now under review) and MESOS-1248
>>>>> (which should be quick) committed and then rebase MESOS-336 in smaller
>>>>> pieces. If you have small pieces, too, I do think we can do this somewhat
>>>>> concurrently, as your refactoring seems to affect mostly how the
>>>>> transport part works. I am on the other hand mostly interested in the
>>>>> bookkeeping of what is in progress, what succeeded, what failed, what has
>>>>> been put where, and so on, which can be separated, if we are careful
>>>>> about it.
>>>>>
>>>>> (BTW, I think we need both unit tests and integration tests. And we don’t
>>>>> have nearly enough of either.)
>>>>>
>>>>> Bernd
>>>>>
>>>>>> On Nov 3, 2014, at 6:18 PM, Adam Bordelon <a...@mesosphere.io
>>>>>> <mailto:a...@mesosphere.io><mailto:a...@mesosphere.io
>>>>>> <mailto:a...@mesosphere.io>>> wrote:
>>>>>>
>>>>>> + Bernd, who has done some fetcher work, including additional testing,
>>>>>> for MESOS-1316, MESOS-1945, and MESOS-336
>>>>>>
>>>>>> On Mon, Nov 3, 2014 at 9:04 AM, Dominic Hamon <dha...@twopensource.com
>>>>>> <mailto:dha...@twopensource.com><mailto:dha...@twopensource.com
>>>>>> <mailto:dha...@twopensource.com>>> wrote:
>>>>>> Hi Ankur
>>>>>>
>>>>>> I think this is a great approach. It makes the code much simpler,
>>>>>> extensible, and more testable. Anyone that's heard me rant knows I am a
>>>>>> big fan of unit tests over integration tests, so this shouldn't surprise
>>>>>> anyone :)
>>>>>>
>>>>>> If you haven't already, please read the documentation on contributing to
>>>>>> Mesos and the style guide to ensure all the naming is as expected, then
>>>>>> you can push the patch to reviewboard to get it reviewed and committed.
>>>>>>
>>>>>> On Mon, Nov 3, 2014 at 12:49 AM, Ankur Chauhan <an...@malloc64.com
>>>>>> <mailto:an...@malloc64.com><mailto:an...@malloc64.com
>>>>>> <mailto:an...@malloc64.com>>> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I did some learning today! This is pretty much a very rough draft of the
>>>>>> tests/refactor of mesos-fetcher that I have come up with. Again, If
>>>>>> there are some obvious mistakes, please let me know. (this is my first
>>>>>> pass after all).
>>>>>> https://github.com/ankurcha/mesos/compare/prefer_2
>>>>>> <https://github.com/ankurcha/mesos/compare/prefer_2><https://github.com/ankurcha/mesos/compare/prefer_2
>>>>>> <https://github.com/ankurcha/mesos/compare/prefer_2>>
>>>>>>
>>>>>> My main intention is to break the logic of the fetcher info some very
>>>>>> discrete components that i can write tests against. I am still
>>>>>> re-learning cpp/mesos code styles etc so I may be a little slow to catch
>>>>>> up but I would really appreciate any comments and/or suggestions.
>>>>>>
>>>>>> -- Ankur
>>>>>> @ankurcha
>>>>>>
>>>>>>> On 2 Nov 2014, at 18:17, Ankur Chauhan <an...@malloc64.com
>>>>>>> <mailto:an...@malloc64.com><mailto:an...@malloc64.com
>>>>>>> <mailto:an...@malloc64.com>>> wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> I noticed that the current set of tests in
>>>>>>> `src/tests/fetcher_tests.cpp` is pretty coarse grained and are more on
>>>>>>> the lines of a functional test. I was going to add some tests but it
>>>>>>> seems like if I am to do that I would need to add a test dependency on
>>>>>>> hadoop.
>>>>>>>
>>>>>>> As an alternative, I propose adding a good set of unit tests around the
>>>>>>> methods used by `src/launcher/fetcher.cpp` and `src/hdfs/hdfs.cpp`.
>>>>>>> This should be able to catch a good portion of cases at the same time
>>>>>>> keeping the dependencies and runtime of tests low. What do you guys
>>>>>>> thing about this?
>>>>>>>
>>>>>>> PS: I am pretty green in terms of gtest and the overall c++ testing
>>>>>>> methodology. Can someone give me pointers to good examples of tests in
>>>>>>> the codebase.
>>>>>>>
>>>>>>> -- Ankur
>>>>>>>
>>>>>>>> On 1 Nov 2014, at 22:54, Adam Bordelon <a...@mesosphere.io
>>>>>>>> <mailto:a...@mesosphere.io><mailto:a...@mesosphere.io
>>>>>>>> <mailto:a...@mesosphere.io>>> wrote:
>>>>>>>>
>>>>>>>> Thank you Ankur. At first glance, it looks great. We'll do a more
>>>>>>>> thorough review of it very soon.
>>>>>>>> I know Tim St. Clair had some ideas for fixing MESOS-1711
>>>>>>>> <https://issues.apache.org/jira/browse/MESOS-1711
>>>>>>>> <https://issues.apache.org/jira/browse/MESOS-1711>>; he may want to
>>>>>>>> review too.
>>>>>>>>
>>>>>>>> On Sat, Nov 1, 2014 at 8:49 PM, Ankur Chauhan <an...@malloc64.com
>>>>>>>> <mailto:an...@malloc64.com><mailto:an...@malloc64.com
>>>>>>>> <mailto:an...@malloc64.com>>> wrote:
>>>>>>>> Hi Tim,
>>>>>>>>
>>>>>>>> I just created a review https://reviews.apache.org/r/27483/
>>>>>>>> <https://reviews.apache.org/r/27483/><https://reviews.apache.org/r/27483/
>>>>>>>> <https://reviews.apache.org/r/27483/>> It's my first stab at it and I
>>>>>>>> will try to add more tests as I figure out how to do the hadoop
>>>>>>>> mocking and stuff. Have a look and let me know what you think about it
>>>>>>>> so far.
>>>>>>>>
>>>>>>>> -- Ankur
>>>>>>>>
>>>>>>>>> On 1 Nov 2014, at 20:05, Ankur Chauhan <an...@malloc64.com
>>>>>>>>> <mailto:an...@malloc64.com><mailto:an...@malloc64.com
>>>>>>>>> <mailto:an...@malloc64.com>>> wrote:
>>>>>>>>>
>>>>>>>>> Yea, i saw that the minute i pressed send. I'll start the review
>>>>>>>>> board so that people can have a look at the change.
>>>>>>>>>
>>>>>>>>> -- Ankur
>>>>>>>>>
>>>>>>>>>> On 1 Nov 2014, at 20:01, Tim Chen <t...@mesosphere.io
>>>>>>>>>> <mailto:t...@mesosphere.io><mailto:t...@mesosphere.io
>>>>>>>>>> <mailto:t...@mesosphere.io>>> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Ankur,
>>>>>>>>>>
>>>>>>>>>> There is a fetcher_tests.cpp in src/tests.
>>>>>>>>>>
>>>>>>>>>> Tim
>>>>>>>>>>
>>>>>>>>>> On Sat, Nov 1, 2014 at 7:27 PM, Ankur Chauhan <an...@malloc64.com
>>>>>>>>>> <mailto:an...@malloc64.com><mailto:an...@malloc64.com
>>>>>>>>>> <mailto:an...@malloc64.com>>> wrote:
>>>>>>>>>> Hi Tim,
>>>>>>>>>>
>>>>>>>>>> I am trying to find/write some test cases. I couldn't find a
>>>>>>>>>> fetcher_tests.{cpp|hpp} so once I have something, I'll post on
>>>>>>>>>> review board. I am new to gmock/gtest so bear with me while i get up
>>>>>>>>>> to speed.
>>>>>>>>>>
>>>>>>>>>> -- Ankur
>>>>>>>>>>
>>>>>>>>>>> On 1 Nov 2014, at 19:23, Timothy Chen <t...@mesosphere.io
>>>>>>>>>>> <mailto:t...@mesosphere.io><mailto:t...@mesosphere.io
>>>>>>>>>>> <mailto:t...@mesosphere.io>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Ankur,
>>>>>>>>>>>
>>>>>>>>>>> Can you post on reviewboard? We can discuss more about the code
>>>>>>>>>>> there.
>>>>>>>>>>>
>>>>>>>>>>> Tim
>>>>>>>>>>>
>>>>>>>>>>> Sent from my iPhone
>>>>>>>>>>>
>>>>>>>>>>>> On Nov 1, 2014, at 6:29 PM, Ankur Chauhan <an...@malloc64.com
>>>>>>>>>>>> <mailto:an...@malloc64.com><mailto:an...@malloc64.com
>>>>>>>>>>>> <mailto:an...@malloc64.com>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi Tim,
>>>>>>>>>>>>
>>>>>>>>>>>> I don't think there is an issue which is directly in line with
>>>>>>>>>>>> what i wanted but the closest one that I could find in JIRA is
>>>>>>>>>>>> https://issues.apache.org/jira/browse/MESOS-1711
>>>>>>>>>>>> <https://issues.apache.org/jira/browse/MESOS-1711><https://issues.apache.org/jira/browse/MESOS-1711
>>>>>>>>>>>> <https://issues.apache.org/jira/browse/MESOS-1711>>
>>>>>>>>>>>>
>>>>>>>>>>>> I have a branch (
>>>>>>>>>>>> https://github.com/ankurcha/mesos/compare/prefer_hadoop_fetcher
>>>>>>>>>>>> <https://github.com/ankurcha/mesos/compare/prefer_hadoop_fetcher><https://github.com/ankurcha/mesos/compare/prefer_hadoop_fetcher
>>>>>>>>>>>>
>>>>>>>>>>>> <https://github.com/ankurcha/mesos/compare/prefer_hadoop_fetcher>>
>>>>>>>>>>>> ) that has a change that would enable users to specify whatever
>>>>>>>>>>>> hdfs compatible uris to the mesos-fetcher but maybe you can weight
>>>>>>>>>>>> in on it. Do you think this is the right track? if so, i would
>>>>>>>>>>>> like to pick this issue and submit a patch for review.
>>>>>>>>>>>>
>>>>>>>>>>>> -- Ankur
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>> On 1 Nov 2014, at 04:32, Tom Arnfeld <t...@duedil.com
>>>>>>>>>>>>> <mailto:t...@duedil.com><mailto:t...@duedil.com
>>>>>>>>>>>>> <mailto:t...@duedil.com>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Completely +1 to this. There are now quite a lot of hadoop
>>>>>>>>>>>>> compatible filesystem wrappers out in the wild and this would
>>>>>>>>>>>>> certainly be very useful.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I'm happy to contribute a patch. Here's a few related issues that
>>>>>>>>>>>>> might be of interest;
>>>>>>>>>>>>>
>>>>>>>>>>>>> - https://issues.apache.org/jira/browse/MESOS-1887
>>>>>>>>>>>>> <https://issues.apache.org/jira/browse/MESOS-1887><https://issues.apache.org/jira/browse/MESOS-1887
>>>>>>>>>>>>> <https://issues.apache.org/jira/browse/MESOS-1887>>
>>>>>>>>>>>>> - https://issues.apache.org/jira/browse/MESOS-1316
>>>>>>>>>>>>> <https://issues.apache.org/jira/browse/MESOS-1316><https://issues.apache.org/jira/browse/MESOS-1316
>>>>>>>>>>>>> <https://issues.apache.org/jira/browse/MESOS-1316>>
>>>>>>>>>>>>> - https://issues.apache.org/jira/browse/MESOS-336
>>>>>>>>>>>>> <https://issues.apache.org/jira/browse/MESOS-336><https://issues.apache.org/jira/browse/MESOS-336
>>>>>>>>>>>>> <https://issues.apache.org/jira/browse/MESOS-336>>
>>>>>>>>>>>>> - https://issues.apache.org/jira/browse/MESOS-1248
>>>>>>>>>>>>> <https://issues.apache.org/jira/browse/MESOS-1248><https://issues.apache.org/jira/browse/MESOS-1248
>>>>>>>>>>>>> <https://issues.apache.org/jira/browse/MESOS-1248>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 31 October 2014 22:39, Tim Chen <t...@mesosphere.io
>>>>>>>>>>>>> <mailto:t...@mesosphere.io><mailto:t...@mesosphere.io
>>>>>>>>>>>>> <mailto:t...@mesosphere.io>>> wrote:
>>>>>>>>>>>>> I believe there is already a JIRA ticket for this, if you search
>>>>>>>>>>>>> for fetcher in Mesos JIRA I think you can find it.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Tim
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Fri, Oct 31, 2014 at 3:27 PM, Ankur Chauhan
>>>>>>>>>>>>> <an...@malloc64.com
>>>>>>>>>>>>> <mailto:an...@malloc64.com><mailto:an...@malloc64.com
>>>>>>>>>>>>> <mailto:an...@malloc64.com>>> wrote:
>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I have been looking at some of the stuff around the fetcher and
>>>>>>>>>>>>> saw something interesting. The code for fetcher::fetch method is
>>>>>>>>>>>>> dependent on a hard coded list of url schemes. No doubt that this
>>>>>>>>>>>>> works but is very restrictive.
>>>>>>>>>>>>> Hadoop/HDFS in general is pretty flexible when it comes to being
>>>>>>>>>>>>> able to fetch stuff from urls and has the ability to fetch a
>>>>>>>>>>>>> large number of types of urls and can be extended by adding
>>>>>>>>>>>>> configuration into the conf/hdfs-site.xml and core-site.xml
>>>>>>>>>>>>>
>>>>>>>>>>>>> What I am proposing is that we refactor the fetcher.cpp to prefer
>>>>>>>>>>>>> to use the hdfs (using hdfs/hdfs.hpp) to do all the fetching if
>>>>>>>>>>>>> HADOOP_HOME is set and $HADOOP_HOME/bin/hadoop is available. This
>>>>>>>>>>>>> logic already exists and we can just use it. The fallback logic
>>>>>>>>>>>>> for using net::download or local file copy is may be left in
>>>>>>>>>>>>> place for installations that do not have hadoop configured. This
>>>>>>>>>>>>> means that if hadoop is present we can directly fetch urls such
>>>>>>>>>>>>> as tachyon://... snackfs:// ... cfs:// .... ftp://... s3://...
>>>>>>>>>>>>> http:// ... file:// with no extra effort. This makes up for a
>>>>>>>>>>>>> much better experience when it comes to debugging and
>>>>>>>>>>>>> extensibility.
>>>>>>>>>>>>>
>>>>>>>>>>>>> What do others think about this?
>>>>>>>>>>>>>
>>>>>>>>>>>>> - Ankur
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Dominic Hamon | @mrdo | Twitter
>>>>>> There are no bad ideas; only good ideas that go horribly wrong.
>>
