You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by aljoscha <gi...@git.apache.org> on 2018/04/18 10:45:28 UTC

[GitHub] flink pull request #5868: [FLINK-9193] Deprecate non-well-defined output met...

GitHub user aljoscha opened a pull request:

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

    [FLINK-9193] Deprecate non-well-defined output methods on DataStream

    

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

    $ git pull https://github.com/aljoscha/flink master

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

    https://github.com/apache/flink/pull/5868.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 #5868
    
----
commit e5c24ea8e6700d65dbb794c5055f0bc40b8fa55f
Author: Aljoscha Krettek <al...@...>
Date:   2018-04-17T13:36:55Z

    Remove unsed Sink Functions

commit fb0ecf646d4fe132d0b0ef18334ff9b9fea0db35
Author: Aljoscha Krettek <al...@...>
Date:   2018-04-17T13:37:29Z

    [FLINK-9193] Deprecate non-well-defined output methods on DataStream

----


---

[GitHub] flink pull request #5868: [FLINK-9193] Deprecate non-well-defined output met...

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

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


---

[GitHub] flink pull request #5868: [FLINK-9193] Deprecate non-well-defined output met...

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

    https://github.com/apache/flink/pull/5868#discussion_r182387176
  
    --- Diff: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/WriteSinkFunctionByMillis.java ---
    @@ -1,53 +0,0 @@
    -/*
    - * Licensed to the Apache Software Foundation (ASF) under one or more
    - * contributor license agreements.  See the NOTICE file distributed with
    - * this work for additional information regarding copyright ownership.
    - * The ASF licenses this file to You under the Apache License, Version 2.0
    - * (the "License"); you may not use this file except in compliance with
    - * the License.  You may obtain a copy of the License at
    - *
    - *    http://www.apache.org/licenses/LICENSE-2.0
    - *
    - * Unless required by applicable law or agreed to in writing, software
    - * distributed under the License is distributed on an "AS IS" BASIS,
    - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    - * See the License for the specific language governing permissions and
    - * limitations under the License.
    - */
    -
    -package org.apache.flink.streaming.api.functions.sink;
    -
    -import org.apache.flink.annotation.PublicEvolving;
    -
    -/**
    - * Implementation of WriteSinkFunction. Writes tuples to file in every millis
    - * milliseconds.
    - *
    - * @param <IN>
    - *            Input tuple type
    - */
    -@PublicEvolving
    --- End diff --
    
    I think we should, because they are bogus and no-one should actually use them.
    
    There is already a follow-up issue: https://issues.apache.org/jira/browse/FLINK-9195 😅 


---

[GitHub] flink issue #5868: [FLINK-9193] Deprecate non-well-defined output methods on...

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

    https://github.com/apache/flink/pull/5868
  
    Created an updated version here: https://github.com/apache/flink/pull/5871


---

[GitHub] flink pull request #5868: [FLINK-9193] Deprecate non-well-defined output met...

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

    https://github.com/apache/flink/pull/5868#discussion_r182385910
  
    --- Diff: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/WriteSinkFunctionByMillis.java ---
    @@ -1,53 +0,0 @@
    -/*
    - * Licensed to the Apache Software Foundation (ASF) under one or more
    - * contributor license agreements.  See the NOTICE file distributed with
    - * this work for additional information regarding copyright ownership.
    - * The ASF licenses this file to You under the Apache License, Version 2.0
    - * (the "License"); you may not use this file except in compliance with
    - * the License.  You may obtain a copy of the License at
    - *
    - *    http://www.apache.org/licenses/LICENSE-2.0
    - *
    - * Unless required by applicable law or agreed to in writing, software
    - * distributed under the License is distributed on an "AS IS" BASIS,
    - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    - * See the License for the specific language governing permissions and
    - * limitations under the License.
    - */
    -
    -package org.apache.flink.streaming.api.functions.sink;
    -
    -import org.apache.flink.annotation.PublicEvolving;
    -
    -/**
    - * Implementation of WriteSinkFunction. Writes tuples to file in every millis
    - * milliseconds.
    - *
    - * @param <IN>
    - *            Input tuple type
    - */
    -@PublicEvolving
    --- End diff --
    
    I don't think we outright delete these.


---

[GitHub] flink pull request #5868: [FLINK-9193] Deprecate non-well-defined output met...

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

    https://github.com/apache/flink/pull/5868#discussion_r182396425
  
    --- Diff: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/WriteSinkFunctionByMillis.java ---
    @@ -1,53 +0,0 @@
    -/*
    - * Licensed to the Apache Software Foundation (ASF) under one or more
    - * contributor license agreements.  See the NOTICE file distributed with
    - * this work for additional information regarding copyright ownership.
    - * The ASF licenses this file to You under the Apache License, Version 2.0
    - * (the "License"); you may not use this file except in compliance with
    - * the License.  You may obtain a copy of the License at
    - *
    - *    http://www.apache.org/licenses/LICENSE-2.0
    - *
    - * Unless required by applicable law or agreed to in writing, software
    - * distributed under the License is distributed on an "AS IS" BASIS,
    - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    - * See the License for the specific language governing permissions and
    - * limitations under the License.
    - */
    -
    -package org.apache.flink.streaming.api.functions.sink;
    -
    -import org.apache.flink.annotation.PublicEvolving;
    -
    -/**
    - * Implementation of WriteSinkFunction. Writes tuples to file in every millis
    - * milliseconds.
    - *
    - * @param <IN>
    - *            Input tuple type
    - */
    -@PublicEvolving
    --- End diff --
    
    I agree that the implementation is horrible. It's inefficient, redundant and untested, but that's unfortunately not really the point here.
    
    They _work_, someone _may_ use them, and they _are_ part of the public API. These methods have always been there, why are we _now_ breaking the API? Why not deprecate them and plaster the constructors with WARN log messages?
    
    I don't see how this change is related to FLINK-9195, given that these classes are completely unused.


---

[GitHub] flink pull request #5868: [FLINK-9193] Deprecate non-well-defined output met...

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

    https://github.com/apache/flink/pull/5868#discussion_r182397973
  
    --- Diff: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/functions/sink/WriteSinkFunctionByMillis.java ---
    @@ -1,53 +0,0 @@
    -/*
    - * Licensed to the Apache Software Foundation (ASF) under one or more
    - * contributor license agreements.  See the NOTICE file distributed with
    - * this work for additional information regarding copyright ownership.
    - * The ASF licenses this file to You under the Apache License, Version 2.0
    - * (the "License"); you may not use this file except in compliance with
    - * the License.  You may obtain a copy of the License at
    - *
    - *    http://www.apache.org/licenses/LICENSE-2.0
    - *
    - * Unless required by applicable law or agreed to in writing, software
    - * distributed under the License is distributed on an "AS IS" BASIS,
    - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    - * See the License for the specific language governing permissions and
    - * limitations under the License.
    - */
    -
    -package org.apache.flink.streaming.api.functions.sink;
    -
    -import org.apache.flink.annotation.PublicEvolving;
    -
    -/**
    - * Implementation of WriteSinkFunction. Writes tuples to file in every millis
    - * milliseconds.
    - *
    - * @param <IN>
    - *            Input tuple type
    - */
    -@PublicEvolving
    --- End diff --
    
    I get your point. Would you be ok with opening a separate Jira issue, deprecating them now and removing in 1.6?
    
    I was always under the impression that we would remove `@PublicEvolving` stuff if necessary but the description of `@PublicEvolving` seems to indicate something else. 😓 


---