You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@streams.apache.org by steveblackmon <gi...@git.apache.org> on 2014/09/12 22:57:50 UTC
[GitHub] incubator-streams pull request: Made linesPerFile configurable
GitHub user steveblackmon opened a pull request:
https://github.com/apache/incubator-streams/pull/82
Made linesPerFile configurable
Made configurator more sensible
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-streams STREAMS-163
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-streams/pull/82.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #82
----
commit 669194f5cd8d59f518d7cdbcab5b66cee9fe56b0
Author: Steve Blackmon <sb...@w2odigital.com>
Date: 2014-09-07T19:13:07Z
Made linesPerFile configurable
Made configurator more sensible
----
---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-streams pull request: Made linesPerFile configurable
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-streams/pull/82
---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
Re: [GitHub] incubator-streams pull request: Made linesPerFile configurable
Posted by Steve Blackmon <sb...@apache.org>.
I've created https://issues.apache.org/jira/browse/STREAMS-186 to
encompass the ideas in the comments. I'd like to merge PR #82
as it stands.
Steve Blackmon
sblackmon@apache.org
On Tue, Oct 7, 2014 at 9:19 AM, mfranklin <gi...@git.apache.org> wrote:
> Github user mfranklin commented on the pull request:
>
> https://github.com/apache/incubator-streams/pull/82#issuecomment-58191330
>
> what is the decision on this?
>
>
> ---
> 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, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
Re: [GitHub] incubator-streams pull request: Made linesPerFile configurable
Posted by Steve Blackmon <st...@blackmon.org>.
I've created https://issues.apache.org/jira/browse/STREAMS-186 to
encompass the ideas in the comments. I'd like to merge the PR as it
stands.
On Tue, Oct 7, 2014 at 9:19 AM, mfranklin <gi...@git.apache.org> wrote:
> Github user mfranklin commented on the pull request:
>
> https://github.com/apache/incubator-streams/pull/82#issuecomment-58191330
>
> what is the decision on this?
>
>
> ---
> 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, or if the feature is enabled but not working, please
> contact infrastructure at infrastructure@apache.org or file a JIRA ticket
> with INFRA.
> ---
[GitHub] incubator-streams pull request: Made linesPerFile configurable
Posted by mfranklin <gi...@git.apache.org>.
Github user mfranklin commented on the pull request:
https://github.com/apache/incubator-streams/pull/82#issuecomment-58191330
what is the decision on this?
---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-streams pull request: Made linesPerFile configurable
Posted by renato2099 <gi...@git.apache.org>.
Github user renato2099 commented on the pull request:
https://github.com/apache/incubator-streams/pull/82#issuecomment-58209096
Cool, maybe I can chip in updating the other modules :)
---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-streams pull request: Made linesPerFile configurable
Posted by mfranklin <gi...@git.apache.org>.
Github user mfranklin commented on the pull request:
https://github.com/apache/incubator-streams/pull/82#issuecomment-58197121
:+1: With the issue created.
---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-streams pull request: Made linesPerFile configurable
Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on the pull request:
https://github.com/apache/incubator-streams/pull/82#issuecomment-56698078
I don't believe it's necessary to test every individual configurator - they should all be doing exactly the same thing - using TypeSafe concise rendering and Jackson to initialize a bean from a fragment of the combined json/hocon/property-file global typesafe config.
I would support adding a class to streams-config to handle this on behalf of all the module so there would just be one central place to tweak config parsing behavior in the future and adding some tests there.
---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-streams pull request: Made linesPerFile configurable
Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on the pull request:
https://github.com/apache/incubator-streams/pull/82#issuecomment-58196837
I've created https://issues.apache.org/jira/browse/STREAMS-186 to
encompass the ideas in the comments. I'd like to merge PR #82
as it stands.
---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-streams pull request: Made linesPerFile configurable
Posted by renato2099 <gi...@git.apache.org>.
Github user renato2099 commented on the pull request:
https://github.com/apache/incubator-streams/pull/82#issuecomment-58197188
Hi Steve,
Why don't creating separate issue per module that uses this new configuration scheme? And let PR#82 be with the one associated with updating HDFS? And it would make sense that the tests only go in the centralized configuration manager class. I am :+1: to merge this if it is done that way.
---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-streams pull request: Made linesPerFile configurable
Posted by robdouglas <gi...@git.apache.org>.
Github user robdouglas commented on the pull request:
https://github.com/apache/incubator-streams/pull/82#issuecomment-56579968
Do we have any tests in place to ensure the configurator is correctly translating the configuration?
---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-streams pull request: Made linesPerFile configurable
Posted by steveblackmon <gi...@git.apache.org>.
Github user steveblackmon commented on the pull request:
https://github.com/apache/incubator-streams/pull/82#issuecomment-58208458
that makes sense, i will add sub-tasks for every module that has a configurator I think can be deprecated with this approach
---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-streams pull request: Made linesPerFile configurable
Posted by renato2099 <gi...@git.apache.org>.
Github user renato2099 commented on the pull request:
https://github.com/apache/incubator-streams/pull/82#issuecomment-56739909
Hi Steve, I also think that having a single class inside a streams-config to handle this in a centralized part would be the way to go, because having other classes doing almost the same thing makes it hard to maintain and homogenize.
---
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, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---