You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@heron.apache.org by GitBox <gi...@apache.org> on 2022/05/30 01:45:04 UTC
[GitHub] [incubator-heron] nicknezis opened a new pull request, #3838: Removed random long in filename which caused leaking in upload storage
nicknezis opened a new pull request, #3838:
URL: https://github.com/apache/incubator-heron/pull/3838
Fixes #3769
--
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: commits-unsubscribe@heron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nicknezis commented on pull request #3838: Removed random long in filename which caused leaking in upload storage
Posted by GitBox <gi...@apache.org>.
nicknezis commented on PR #3838:
URL: https://github.com/apache/incubator-heron/pull/3838#issuecomment-1140603388
Also reverting [this change](https://github.com/apache/incubator-heron/pull/3628) which was added 2 years ago as a bandaid to remedy #3769 .
--
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: commits-unsubscribe@heron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nicknezis commented on pull request #3838: Removed random long in filename which caused leaking in upload storage
Posted by GitBox <gi...@apache.org>.
nicknezis commented on PR #3838:
URL: https://github.com/apache/incubator-heron/pull/3838#issuecomment-1140626226
> The commit looks good. Just wonder what was the original reason of using a random name in bookkeeper upload?
It looks like a bit of code added early in this [PR](https://github.com/apache/incubator-heron/pull/601). Someone suggested they use a timestamp instead of random to see which package was newest. But it doesn't seem to have much thought put behind the original Random Long. I'll have to look further back in the history to determine when it was added.
--
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: commits-unsubscribe@heron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nwangtw commented on pull request #3838: Removed random long in filename which caused leaking in upload storage
Posted by GitBox <gi...@apache.org>.
nwangtw commented on PR #3838:
URL: https://github.com/apache/incubator-heron/pull/3838#issuecomment-1140785195
Based on the unit test: `testGenerateFilename`, it seems like the file name is expected to be unique.
--
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: commits-unsubscribe@heron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] windhamwong commented on pull request #3838: Removed random long in filename which caused leaking in upload storage
Posted by GitBox <gi...@apache.org>.
windhamwong commented on PR #3838:
URL: https://github.com/apache/incubator-heron/pull/3838#issuecomment-1140619426
The commit looks good. Just wonder what was the original reason of using a random name in bookkeeper upload?
--
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: commits-unsubscribe@heron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nwangtw commented on pull request #3838: Removed random long in filename which caused leaking in upload storage
Posted by GitBox <gi...@apache.org>.
nwangtw commented on PR #3838:
URL: https://github.com/apache/incubator-heron/pull/3838#issuecomment-1141369208
Yeah. It sounds a little strange. Maybe the logic is assuming manual cleanup or some kind of TTL?
--
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: commits-unsubscribe@heron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nicknezis commented on pull request #3838: Removed random long in filename which caused leaking in upload storage
Posted by GitBox <gi...@apache.org>.
nicknezis commented on PR #3838:
URL: https://github.com/apache/incubator-heron/pull/3838#issuecomment-1141305876
> Based on the unit test: `testGenerateFilename`, it seems like the file name is expected to be unique.
Interesting, the test still passes. I'll have to dig into it further. But looking in each Uploader, they seem to have logic for uploading a package with the same name. But this logic would never be needed if the intention was to always generate a unique filename. I really think this is a relic from the old Twitter code base. It doesn't seem to provide any value.
--
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: commits-unsubscribe@heron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-heron] nicknezis merged pull request #3838: Removed random long in filename which caused leaking in upload storage
Posted by GitBox <gi...@apache.org>.
nicknezis merged PR #3838:
URL: https://github.com/apache/incubator-heron/pull/3838
--
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: commits-unsubscribe@heron.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org