Centralize partitioning, fix RobotRules
Hello @jnioche,
I hope I don't overdo it with my PRs, but I really did a lot in the last few months. ^^' So many thanks in advance for your patience, support and help. :)
As the title says this PR aims to clean up the decentralized handling of the partitioning keys and parsing logic.
In order to reach that goal I removed 3 different parsing/handling implementations of the very same feature and unified it in a single implementation in PartitionMode and PartitionUtil.
The syntax of the config doesn't change, only the places where it is parsed and handled. Furthermore I improved the way we handle the interpretation of the values because I copied the most stable implementation of the three and used it as a base for the new one.
The pros of this PR are:
- Removes multiple implementations for the same logic and replaces it with a single, reliable one.
- Remove redeclaration of the same constant string values (
byHostetc.) in multiple files. - Reduces the overall loc and makes the rest of the functions clearer to the reader.
- Improve security and stability of code by unifying the interpretation of the partitioning keys.
Furthermore I fixed a bug where the equals method of RobotRules always fails.
Best Regards
Felix
Developer Certificate of Origin
By contributing to StormCrawler, you accept and agree to the following terms and conditions (the Developer Certificate of Origin) for your present and future contributions submitted to StormCrawler.
Please refer to the Developer Certificate of Origin section in CONTRIBUTING.md for details.
Developer Certificate of Origin
Version 1.1
Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129
Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
hi @FelixEngl
I hope I don't overdo it with my PRs, but I really did a lot in the last few months. ^^'
You are certainly prolific!
So many thanks in advance for your patience, support and help. :)
you are welcome. I am very grateful for your contributions and being open minded about our suggestions
I won't have time to look at the last PRs in the short term - am running behind on some other tasks but I will review them next week. I think some of them involve loads of files: if a PR does more than one thing and there is any chance of splitting it, that would really help. The effort it takes to review a PR compared to its length is probably quadratic ;-)
hi @FelixEngl
I hope I don't overdo it with my PRs, but I really did a lot in the last few months. ^^'
You are certainly prolific!
So many thanks in advance for your patience, support and help. :)
you are welcome. I am very grateful for your contributions and being open minded about our suggestions
I won't have time to look at the last PRs in the short term - am running behind on some other tasks but I will review them next week. I think some of them involve loads of files: if a PR does more than one thing and there is any chance of splitting it, that would really help. The effort it takes to review a PR compared to its length is probably quadratic ;-)
Ok I'll do that better in the next PRs. Do you want me to split the PR up or are you fine with this one as is?
hi @FelixEngl
I hope I don't overdo it with my PRs, but I really did a lot in the last few months. ^^'
You are certainly prolific!
So many thanks in advance for your patience, support and help. :)
you are welcome. I am very grateful for your contributions and being open minded about our suggestions I won't have time to look at the last PRs in the short term - am running behind on some other tasks but I will review them next week. I think some of them involve loads of files: if a PR does more than one thing and there is any chance of splitting it, that would really help. The effort it takes to review a PR compared to its length is probably quadratic ;-)
Ok I'll do that better in the next PRs. Do you want me to split the PR up or are you fine with this one as is?
@jnioche I think I could split this PR up (basically I just have to move that overridden equals to an other branch). But I splitting up #948 could be quite hard, because the new proposed feature requires the changes to the ConfUtils.
hi @FelixEngl
I hope I don't overdo it with my PRs, but I really did a lot in the last few months. ^^'
You are certainly prolific!
So many thanks in advance for your patience, support and help. :)
you are welcome. I am very grateful for your contributions and being open minded about our suggestions I won't have time to look at the last PRs in the short term - am running behind on some other tasks but I will review them next week. I think some of them involve loads of files: if a PR does more than one thing and there is any chance of splitting it, that would really help. The effort it takes to review a PR compared to its length is probably quadratic ;-)
Ok I'll do that better in the next PRs. Do you want me to split the PR up or are you fine with this one as is?
@jnioche I think I could split this PR up (basically I just have to move that overridden
equalsto an other branch). But I splitting up #948 could be quite hard, because the new proposed feature requires the changes to theConfUtils.
This one has only 10 files to review, no worries
This PR is probably too big and hasn't had any activity for nearly a year so I am closing it.