You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by ajaybhat <gi...@git.apache.org> on 2016/01/07 10:07:03 UTC

[GitHub] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

GitHub user ajaybhat opened a pull request:

    https://github.com/apache/flink/pull/1491

    [FLINK-2125][streaming] Delimiter change from char to string

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ajaybhat/flink char_buffer

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/1491.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 #1491
    
----
commit 857fec66f8373a8537232ff1a748962ff5bbe671
Author: Ajay Bhat <a....@gmail.com>
Date:   2016-01-07T06:54:58Z

    [FLINK-2125][streaming] Delimiter change from char to string

----


---
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] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the pull request:

    https://github.com/apache/flink/pull/1491#issuecomment-170043300
  
    Thanks for your contribution @ajaybhat.
    
    The PR contains many whitespace changes. Would be good to avoid them.
    
    I had a question concerning the implementation. I might be the case that the text after the last delimiter is discarded.


---
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] flink pull request #1491: [FLINK-2125][streaming] Delimiter change from char...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/flink/pull/1491


---
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] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by rmetzger <gi...@git.apache.org>.
Github user rmetzger commented on the pull request:

    https://github.com/apache/flink/pull/1491#issuecomment-169659563
  
    Looks like the tests are failing.


---
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] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the pull request:

    https://github.com/apache/flink/pull/1491#issuecomment-170854651
  
    This change breaks the API and should be done with care.
    Please add new constructors for the `String` delimiter and deprecate the existing constructors with the `char` parameter. 
    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 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] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by aljoscha <gi...@git.apache.org>.
Github user aljoscha commented on the pull request:

    https://github.com/apache/flink/pull/1491#issuecomment-176839361
  
    @ajaybhat Do you have any updates on this PR?


---
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] flink issue #1491: [FLINK-2125][streaming] Delimiter change from char to str...

Posted by fhueske <gi...@git.apache.org>.
Github user fhueske commented on the issue:

    https://github.com/apache/flink/pull/1491
  
    Hi @ajaybhat, do you plan to update this PR? Otherwise, I'd like to close it.


---
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] flink issue #1491: [FLINK-2125][streaming] Delimiter change from char to str...

Posted by greghogan <gi...@git.apache.org>.
Github user greghogan commented on the issue:

    https://github.com/apache/flink/pull/1491
  
    This PR should be closed as FLINK-2125 was fixed with #2233.


---
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] flink pull request: [FLINK-2125][streaming] Delimiter change from ...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/1491#discussion_r49204456
  
    --- Diff: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/source/SocketTextStreamFunction.java ---
    @@ -84,25 +84,25 @@ public SocketTextStreamFunction(String hostname, int port, char delimiter, long
     	public void run(SourceContext<String> ctx) throws Exception {
     		final StringBuilder buffer = new StringBuilder();
     		long attempt = 0;
    -		
    +
     		while (isRunning) {
    -			
    +
     			try (Socket socket = new Socket()) {
     				currentSocket = socket;
    -				
    +
     				LOG.info("Connecting to server socket " + hostname + ':' + port);
     				socket.connect(new InetSocketAddress(hostname, port), CONNECTION_TIMEOUT_TIME);
     				BufferedReader reader = new BufferedReader(new InputStreamReader(socket.getInputStream()));
     
    -				int data;
    -				while (isRunning && (data = reader.read()) != -1) {
    -					// check if the string is complete
    -					if (data != delimiter) {
    -						buffer.append((char) data);
    -					}
    -					else {
    -						// truncate trailing carriage return
    -						if (delimiter == '\n' && buffer.length() > 0 && buffer.charAt(buffer.length() - 1) == '\r') {
    +				char[] charBuffer = new char[Math.max(8192,delimiter.length()*2)];
    +				int bytesRead;
    +				while (isRunning && (bytesRead = reader.read(charBuffer)) != -1) {
    +					String input = new String(charBuffer, 0, bytesRead);
    +					int start = 0,pos;
    +					while ((pos = input.indexOf(delimiter, start)) > -1) {
    --- End diff --
    
    What happens if `input` does not contain `delimiter`? And what happens with the input part after the last delimiter?


---
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.
---