Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new bot: cut string from string #1965

Merged
12 commits merged into from
Sep 24, 2021

Conversation

mariuskarotkis
Copy link
Contributor

Add new bot: cut string from string

@ghost ghost added component: bots feature request Indicates new feature requests labels Jun 7, 2021
@ghost ghost self-requested a review June 7, 2021 15:20
ghost
ghost previously requested changes Jun 7, 2021
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add documentation in docs/user/bots.rst

@codecov-commenter
Copy link

codecov-commenter commented Jun 8, 2021

Codecov Report

Merging #1965 (a3f6d9b) into develop (0750a3d) will increase coverage by 0.31%.
The diff coverage is 91.66%.

❗ Current head a3f6d9b differs from pull request most recent head 4f804c7. Consider uploading reports for the commit 4f804c7 to get more accurate results

@@             Coverage Diff             @@
##           develop    #1965      +/-   ##
===========================================
+ Coverage    75.74%   76.06%   +0.31%     
===========================================
  Files          414      434      +20     
  Lines        22169    23189    +1020     
  Branches      2944     3099     +155     
===========================================
+ Hits         16793    17639     +846     
- Misses        4697     4841     +144     
- Partials       679      709      +30     
Impacted Files Coverage Δ
intelmq/bots/experts/cut_from_string/expert.py 82.60% <82.60%> (ø)
.../tests/bots/experts/cut_from_string/test_expert.py 100.00% <100.00%> (ø)
intelmq/tests/bots/experts/wait/test_expert.py 50.00% <0.00%> (-50.00%) ⬇️
intelmq/bots/experts/wait/expert.py 48.57% <0.00%> (-31.43%) ⬇️
intelmq/bots/parsers/cert_eu/parser_csv.py 78.84% <0.00%> (-2.79%) ⬇️
intelmq/bots/parsers/misp/parser.py 85.45% <0.00%> (-2.79%) ⬇️
intelmq/bots/experts/tor_nodes/expert.py 46.42% <0.00%> (-2.33%) ⬇️
intelmq/bots/collectors/mail/_lib.py 67.27% <0.00%> (-1.96%) ⬇️
intelmq/bots/experts/asn_lookup/expert.py 35.83% <0.00%> (-1.89%) ⬇️
...telmq/bots/experts/recordedfuture_iprisk/expert.py 41.93% <0.00%> (-1.89%) ⬇️
... and 421 more

ghost
ghost previously requested changes Jun 8, 2021
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, the bot will remove a prefix or a suffix from a field. So I would call it RemoveAffix. There is also PEP616 which proposes str methods which do exactly that and which seems to have been accepted for Python 3.9. So once we only support Python 3.9, we can use that (maybe add a comment so that we don't forget).

Please add a reuse compliant license and copyright header to the files, an example can be found in the documentation

intelmq/bots/experts/cut_from_string/expert.py Outdated Show resolved Hide resolved
intelmq/bots/experts/cut_from_string/expert.py Outdated Show resolved Hide resolved
intelmq/bots/experts/cut_from_string/expert.py Outdated Show resolved Hide resolved
@mariuskarotkis
Copy link
Contributor Author

Updated

@ghost ghost added the needs: feedback label Aug 20, 2021
@mariuskarotkis
Copy link
Contributor Author

If I understand correctly, the bot will remove a prefix or a suffix from a field. So I would call it RemoveAffix. There is also PEP616 which proposes str methods which do exactly that and which seems to have been accepted for Python 3.9. So once we only support Python 3.9, we can use that (maybe add a comment so that we don't forget).

Please add a reuse compliant license and copyright header to the files, an example can be found in the documentation

Please check.

@ghost
Copy link

ghost commented Aug 26, 2021

If I understand correctly, the bot will remove a prefix or a suffix from a field. So I would call it RemoveAffix. There is also PEP616 which proposes str methods which do exactly that and which seems to have been accepted for Python 3.9. So once we only support Python 3.9, we can use that (maybe add a comment so that we don't forget).
Please add a reuse compliant license and copyright header to the files, an example can be found in the documentation

Please check.

Thanks, it looks better now from my point of view. There are still a couple of tests that fail, please fix those (apparently the function parameter syntax / was only introduced in Python 3.8, given that we also support 3.6 we have to live without it)

@mariuskarotkis mariuskarotkis requested review from a user August 26, 2021 09:02
@ghost ghost removed their request for review September 7, 2021 06:03
@ghost ghost dismissed their stale review September 17, 2021 11:56

outdated

@ghost
Copy link

ghost commented Sep 17, 2021

I think the only discussion point left open is this comment/proposal by @schacht-certat:

If I understand correctly, the bot will remove a prefix or a suffix from a field. So I would call it RemoveAffix.

@mariuskarotkis What do you think of this idea? Does something indicate against it?

@ghost ghost dismissed their stale review September 17, 2021 12:32

outdated

@mariuskarotkis
Copy link
Contributor Author

RemoveAffix

It's good for me. I think it can be renamed.

@ghost ghost added this to the 3.1.0 milestone Sep 24, 2021
@ghost ghost self-assigned this Sep 24, 2021
@ghost ghost merged commit 6d7ab08 into certtools:develop Sep 24, 2021
@ghost
Copy link

ghost commented Sep 24, 2021

Thank you very much - again! - for this useful addition.

@mariuskarotkis mariuskarotkis deleted the add_bot_cut_from_string branch September 24, 2021 09:07
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants