928 ceilometer statistics date parsing #1049
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Also, what is the release cycle for OS4J? |
|
Ah. Sorry for the delay. Too busy. I'll take a look tomorrow morning. sets a reminder |
|
No problem, I was just wondering if anyone had seen it, or what the procedure was :) Thanks for checking it out |
auhlig
reviewed
Jun 28, 2017
| private Date periodEnd; | ||
|
|
||
| private Double sum; | ||
|
|
||
| private String unit; | ||
|
|
||
| private String groupby; | ||
| @JsonProperty("groupby") | ||
| private Map<String, Object> groupBy; |
There was a problem hiding this comment.
Isn't it justMap<String, String>?
There was a problem hiding this comment.
It actually didn't work with the second type as a String. It was just ignored by Jackson. This field is actually a bit confusing, it looks like:
{"project_id":"some_id"}
It itsn't necessary for this PR so I think I will revert the change.
| @@ -13,19 +19,25 @@ | |||
| * @author Jeremy Unruh | |||
| */ | |||
| public class CeilometerStatistics implements Statistics { | |||
|
|
|||
| private static final Logger LOG = LoggerFactory.getLogger(CeilometerStatistics.class); | |||
There was a problem hiding this comment.
Indentation alternating between spaces and tabs. Could you reformat this file?
| @@ -34,17 +46,22 @@ | |||
|
|
|||
| private Integer period; | |||
|
|
|||
| @JsonProperty("period_start") | |||
| @JsonProperty("period_start") | |||
There was a problem hiding this comment.
Would
@JsonFormat(pattern = "yyyy-MM-dd'T'HH:mm:ss.SSS")
@JsonProperty("period_start")
private Date durationStart;
instead of adding a new property work?
Then you could also revert the changes made to the getDurationStart() and getDurationEnd().
Please add a test for this.
There was a problem hiding this comment.
Cool, didn't know that existed. Will test
|
At least the pipeline runs the tests. |
|
@auhlig I checked out the pipeline, and the tests from core-test are not being run. |
|
There is In the pom.xml of core-test |
|
They are executed for each connector. Scroll further. Then you the the logs from the tests. |
|
Ok, so if I create a test for this, in which project should it be created? In core? |
|
The unit tests can be found under the respective service-folder in the |
|
I reverted the |
|
Sorry. Was about to review but hit close accidentally. |
|
Awesome. Thanks :) |
|
Thanks for contributing @pdube :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
This is my first PR for openstack4j. The dates being parsed for the Statistics are not offset anymore by the microseconds as explained in #928. I am having problems running the core-tests though, each time, I am getting
Tests are skipped.even when adding-Dmaven.test.skip=falseIf someone could give me a hand that would be great