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

BOT: collector ftp(s) #455

Merged
merged 11 commits into from
May 17, 2016
Merged

BOT: collector ftp(s) #455

merged 11 commits into from
May 17, 2016

Conversation

robcza
Copy link
Contributor

@robcza robcza commented Feb 28, 2016

Bots for collecting files from FTP and FTPS, both bots include automatic unzipping

Possible issue is no proxy support

@codecov-io
Copy link

Current coverage is 65.70%

Merging #455 into master will decrease coverage by -0.74% as of c123202

@@            master   #455   diff @@
=====================================
  Files          175    179     +4
  Stmts         6750   6884   +134
  Branches         0      0       
  Methods          0      0       
=====================================
+ Hit           4485   4523    +38
  Partial          0      0       
- Missed        2265   2361    +96

Review entire Coverage Diff as of c123202

Powered by Codecov. Updated on successful CI builds.

@sebix
Copy link
Member

sebix commented Feb 28, 2016

Please write a least a dummy test, so the requirements and syntax will be checked as here: https://github.com/certtools/intelmq/blob/master/intelmq/tests/bots/collectors/alienvault_otx/test_collector.py

@sebix sebix self-assigned this Feb 28, 2016
@sebix sebix added this to the Release 1 - v1.0 milestone Feb 28, 2016
from intelmq.lib.message import Report


# https://stackoverflow.com/questions/12164470/python-ftp-implicit-tls-connection-issue
Copy link
Member

Choose a reason for hiding this comment

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

If you copied code from there, attribution is required under cc by-sa 3.0: https://blog.stackoverflow.com/2009/06/attribution-required/

@robcza
Copy link
Contributor Author

robcza commented Feb 28, 2016

@sebix thank you for the comments, will fix and include the attribution

report.add("feed.url", self.parameters.ftp_host + ':' +
str(self.parameters.ftp_port), sanitize=True)
report.add("feed.accuracy", self.parameters.accuracy, sanitize=True)
time_observation = DateTime().generate_datetime_now()
Copy link
Member

Choose a reason for hiding this comment

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

this line is obsolete

from intelmq.lib.message import Report


# BEGIN content from Stack Overflow
Copy link
Member

Choose a reason for hiding this comment

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

I'd also explicitly add cc by-sa 3.0

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 65.703% when pulling 03e49f0 on robcza:ftp-collector into fac7721 on certtools:master.

@sebix
Copy link
Member

sebix commented Mar 29, 2016

Is it final?

@aaronkaplan
Copy link
Member

aaronkaplan commented May 11, 2016

ping? @robcza . Should we merge? Did you test it?
What about setups which need to go through a proxy?

@sebix sebix removed the incomplete label May 17, 2016
@sebix sebix merged commit 3c9958c into certtools:master May 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants