You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2023/01/09 13:15:53 UTC

[GitHub] [flink] MartijnVisser opened a new pull request, #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink

MartijnVisser opened a new pull request, #21630:
URL: https://github.com/apache/flink/pull/21630

   Same as https://github.com/apache/flink/pull/21371, except for c4dc109cba59f406f2f1a38ed4e3fdadb62d4970 which now also removes the StreamingFileSink from the E2E tests


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] flinkbot commented on pull request #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink

Posted by GitBox <gi...@apache.org>.
flinkbot commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-1375617336

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "c4dc109cba59f406f2f1a38ed4e3fdadb62d4970",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "c4dc109cba59f406f2f1a38ed4e3fdadb62d4970",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * c4dc109cba59f406f2f1a38ed4e3fdadb62d4970 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] MartijnVisser commented on pull request #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink

Posted by GitBox <gi...@apache.org>.
MartijnVisser commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-1380261028

   > Alternatively, if you really want to push for it, it might be more reasonable to ask @JingsongLi for a review or someone more related to whether the partial cleanup is to his liking. WDYT?
   
   The original PR (https://github.com/apache/flink/pull/21371) was reviewed and approved by @gaoyunhaii so it might make more sense to ask him :)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] MartijnVisser commented on pull request #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink

Posted by "MartijnVisser (via GitHub)" <gi...@apache.org>.
MartijnVisser commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-1426319887

   @gaoyunhaii Can you take one more look? You've already reviewed the original PR, which missed one thing that is now addressed in this PR.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] MartijnVisser commented on pull request #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink

Posted by GitBox <gi...@apache.org>.
MartijnVisser commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-1377184327

   @flinkbot run azure


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink [flink]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-2593615243

   This PR is being marked as stale since it has not had any activity in the last 180 days. 
   If you would like to keep this PR alive, please leave a comment asking for a review. 
   If the PR has merge conflicts, update it with the latest from the base branch.
   <p>
   If you are having difficulty finding a reviewer, please reach out to the 
   [community](https://flink.apache.org/what-is-flink/community/).
   <p>
   If this PR is no longer valid or desired, please feel free to close it. 
   If no activity occurs in the next 90 days, it will be automatically closed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] MartijnVisser commented on pull request #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink

Posted by "MartijnVisser (via GitHub)" <gi...@apache.org>.
MartijnVisser commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-1609337377

   > Very sorry for missing the message, I’ll have a look soon.
   
   Thank you!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] MartijnVisser commented on pull request #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink

Posted by GitBox <gi...@apache.org>.
MartijnVisser commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-1377897663

   > Why don't we remove everything at once?
   
   That was the original idea, but I chatted offline with @JingsongLi and there is work to be done on `FileSystemTableSink` in order to be able to completely remove `StreamingFileSink`. I've created https://issues.apache.org/jira/browse/FLINK-30627 as a follow-up ticket. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] XComp commented on pull request #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink

Posted by GitBox <gi...@apache.org>.
XComp commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-1377261399

   What's the plan with the `StreamingFileSink`? Are there follow-up tickets which are not linked with FLINK-30166? I'm just wondering because we're removing tests here for a class that's still present in the codebase. Why don't we remove everything at once?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] XComp commented on pull request #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink

Posted by GitBox <gi...@apache.org>.
XComp commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-1380258947

   Sorry for being picky on that one. But even with knowing these details, it becomes kind of hard for me to review this PR. There are still code location where I have questions but which are hard to mention in this PR if they are actually not touched as code change. Why do we need to do the cleanup before the refactoring? Can't we wait for the refactoring and clean up everything properly afterwards? Or do we gain anything from cleaning up code partially which I'm not aware of?
   
   Alternatively, if you really want to push for it, it might be more reasonable to ask @JingsongLi for a review or someone more related to whether the partial cleanup is to his liking. WDYT?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink [flink]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-2771458141

   This PR has been closed since it has not had any activity in 120 days. 
   If you feel like this was a mistake, or you would like to continue working on it, 
   please feel free to re-open the PR and ask for a review.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink [flink]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink
URL: https://github.com/apache/flink/pull/21630


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] gaoyunhaii commented on pull request #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink

Posted by "gaoyunhaii (via GitHub)" <gi...@apache.org>.
gaoyunhaii commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-1605347451

   Very sorry for missing the message, I’lol have a look soon.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] MartijnVisser commented on pull request #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink

Posted by GitBox <gi...@apache.org>.
MartijnVisser commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-1375906841

   > @MartijnVisser keep in mind that I reverted all the commits related to [FLINK-30166](https://issues.apache.org/jira/browse/FLINK-30166). PR #21371 changed 13 files. This PR only changes 3. I guess, you have to cherry-pick the other commits once more? 🤔
   
   Yeah I forgot to revert your reversion ;) I've done that now, while making the commit message a bit more clear 😅 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [flink] MartijnVisser commented on pull request #21630: [FLINK-30166][Connector/FileSystem] Refactor tests that use the deprecated StreamingFileSink instead of FileSink

Posted by GitBox <gi...@apache.org>.
MartijnVisser commented on PR #21630:
URL: https://github.com/apache/flink/pull/21630#issuecomment-1380023964

   > Should we get rid of those as well?
   
   We can't yet due to https://issues.apache.org/jira/browse/FLINK-30627


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org