Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

[WIP] Feature/use last timestamp #91

Merged
merged 14 commits into from
Sep 18, 2017

Conversation

MojioMS
Copy link

@MojioMS MojioMS commented Sep 8, 2017

integration of usedLastTimeStamp

  • extract code into new Timestamp(String) constructor
  • tests?

@EHJ-52n EHJ-52n changed the title Feature/use last timestamp [WIP] Feature/use last timestamp Sep 8, 2017
@EHJ-52n EHJ-52n added this to the 0.5 RC1 milestone Sep 8, 2017
@EHJ-52n
Copy link
Member

EHJ-52n commented Sep 11, 2017

@MojioMS Please solve the conflicts and tell me when this PR is ready to merge.

Copy link
Member

@EHJ-52n EHJ-52n left a comment

Choose a reason for hiding this comment

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

Please update as requested.

// Get stored lastUsedTimestamp, if it is set:
if (isUseLastTimestamp) {
lastLine = dataFile.getFirstLineWithData();
// TODO Problem gelöst, dass immer auf die erste Zeile mit Daten zurückgesetzt werden muss?
Copy link
Member

Choose a reason for hiding this comment

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

@MojioMS: Please remove german comments!

@@ -342,6 +349,11 @@ public void importData(final DataFile dataFile)
LOG.error("No measured value columns found in configuration");
return;
}
// Get stored lastUsedTimestamp, if it is set:
Copy link
Member

Choose a reason for hiding this comment

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

@MojioMS This comments seems misleading as the next line is setting the lastLine value and not lastUsedTimestamp! Please update the comment or fix the bug.

@@ -396,6 +408,11 @@ public void importData(final DataFile dataFile)
if (!timeSeriesRepository.isEmpty()) {
insertTimeSeries(timeSeriesRepository);
}
if (isUseLastTimestamp
Copy link
Member

Choose a reason for hiding this comment

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

@MojioMS Synchronize with condition of if statement in line 438. Please extract private boolean method with speaking name that replaces the condition in both statements.

}
// store lastUsedTimestamp in configuration/station?
} else {
// abort Insertion
Copy link
Member

Choose a reason for hiding this comment

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

@MojioMS please add a log message here why the observation is skipped.

@@ -1309,4 +1344,24 @@ public void setLastLine(final int lastLine) {
this.lastLine = lastLine;
}

/**
Copy link
Member

Choose a reason for hiding this comment

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

@MojioMS please remove this javadoc.

return lastUsedTimestamp;
}

/**
Copy link
Member

Choose a reason for hiding this comment

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

@MojioMS please remove this javadoc.

// read already inserted UsedLastTimeStamp
LOG.debug("Read already inserted LastUsedTimeStamp from file '{}'.",
timeStampFile.getCanonicalPath());
String storedTimeStamp = null;
Copy link
Member

Choose a reason for hiding this comment

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

@MojioMS remove this line and declare the variable in line 239.

@@ -320,7 +320,20 @@ public boolean isSosTransactional() {
}
return false;
}


/**
Copy link
Member

Choose a reason for hiding this comment

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

please remove javadoc

@EHJ-52n EHJ-52n merged commit 1fdda2a into 52North:develop Sep 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants