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

Fix format in mongoDB bots #1322

Merged
6 commits merged into from
Sep 28, 2018
Merged

Conversation

kalyparker
Copy link
Contributor

Actually date are saved in format string in mongoDB.
This fix change the format for time.observation and time.source from string to ISODate.

Actually date are saved in format string in mongoDB.
This fix change the format for time.observation and time.source from string to ISODate.
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 a changelog entry (for 1.1.0)

And thanks for choosing the correct destination branch :)

@@ -43,7 +44,10 @@ def process(self):
event = self.receive_message()

try:
self.collection.insert(event.to_dict(hierarchical=self.parameters.hierarchical_output))
tmp_dict = event.to_dict(hierarchical=self.parameters.hierarchical_output)
tmp_dict["time"]["observation"] = dateutil.parser.parse(tmp_dict["time"]["observation"])
Copy link

Choose a reason for hiding this comment

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

The existence of time.observation and time.source is assumed here but cannot be guaranteed. See also the failing test.

Copy link

Choose a reason for hiding this comment

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

It is much faster to not use the dateutil parser here because it needs to guess the format based on heuristics. As the format is known, you can use intelmq.lib.harmonization.DateTime.parse_utc_isoformat (I just changed this method from private to public in the maintenance branch)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thks for the tips, I'll change that asap

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, no time this week. I'll do it next week

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unfortunately, using intelmq.lib.harmonization.DateTime.parse_utc_isoformat is not enough. The format returned is still a str. So pymongo insert a string instead of Isodate.
I don't see any way to avoid using dateutil

Copy link

Choose a reason for hiding this comment

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

Oh, ok. That method in the libs looks strange, I need to look at it later when I refactor that file.

@ghost
Copy link

ghost commented Sep 12, 2018

I just thought about moving the logic to intelmq.lib.message.Message.to_dict in the future as this is possibly useful for other outputs too (thinking about postgres for example).

@ghost ghost modified the milestones: 1.2.0, 1.1.1 Sep 25, 2018
@ghost ghost self-assigned this Sep 25, 2018
@ghost ghost added bug Indicates an unexpected problem or unintended behavior component: bots labels Sep 25, 2018
…ls#1322

Add parameters replacement_char for changing default dot separator when
flatten the hierarchy. Fix certtools#1324
@codecov-io
Copy link

codecov-io commented Sep 26, 2018

Codecov Report

Merging #1322 into maintenance will decrease coverage by 0.08%.
The diff coverage is 21.05%.

@@               Coverage Diff               @@
##           maintenance    #1322      +/-   ##
===============================================
- Coverage        75.57%   75.48%   -0.09%     
===============================================
  Files              273      273              
  Lines            12874    12892      +18     
  Branches          1750     1758       +8     
===============================================
+ Hits              9729     9732       +3     
- Misses            2748     2760      +12     
- Partials           397      400       +3
Impacted Files Coverage Δ
intelmq/bots/outputs/mongodb/output.py 49.09% <21.05%> (-15.78%) ⬇️
intelmq/bin/intelmqctl.py 9.48% <0%> (ø) ⬆️

@ghost
Copy link

ghost commented Sep 26, 2018

Why is the replacement char now needed?

@kalyparker
Copy link
Contributor Author

I add the replacement_char for fixing the issue #1324
Should I separate both fix ?

@ghost
Copy link

ghost commented Sep 26, 2018

Ah, that was this one. I am now writing the missing changelog entries and I already forgot about this issue :)

Sebastian Wagner and others added 3 commits September 26, 2018 15:52
parameter replacement_char: handling if not given
style issues
reduce code duplication
@ghost ghost merged commit 9276239 into certtools:maintenance Sep 28, 2018
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior component: bots
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants