openhab / openhab1-addons

Add-ons for openHAB 1.x
Eclipse Public License 2.0
3.43k stars 1.71k forks source link

Enable expiration of data in DynamoDB #5875

Closed mindstorms6 closed 3 years ago

mindstorms6 commented 5 years ago

Howdy! This is a work in progress for enabling expiration of historic data in the DDB Persistence side of OpenHab. DDB has a built in feature that expires items when a ttl is present. My thought process was to

  1. Ensure we create tables with a TTL (if enabled)
  2. For data that is inserted into DDB - add an expiration (epoch seconds) field to it with a 90 day (but configurable) expiration date from the time of data collection.

It's not ready to merge just yet - but I wanted to get feedback from folks in the know (first pull request 😬)

Let me know the obvious things I missed / things you want to see.

Thanks!

mindstorms6 commented 5 years ago

cc @ssalonen

ssalonen commented 5 years ago

@mindstorms6 Great to see new contributor to the DynamoDB addon, welcome! Please see my comments above

ssalonen commented 5 years ago

Also, there are some repo changes coming towards persistence addons: https://github.com/openhab/openhab1-addons/pull/5844 https://github.com/openhab/openhab2-addons/pull/527. https://github.com/openhab/openhab-core/pull/680

Once this is done, you might need to rebase the changes to the repo worked on in those PRs.

Actually, this also means that java 8 is probably ok at least in the new repo structure.

kaikreuzer commented 5 years ago

Actually, this also means that java 8 is probably ok at least in the new repo structure.

Yes, I agree - as the persistence add-ons are about to be moved after the 2.5 release, it should be fine (and actually much preferred) to use Java8 instead of Joda.

mindstorms6 commented 4 years ago

I've addressed the above comment I believe, and added/run the integration tests. You were right - the get/set must be in each child class - TIL.

Marking as "ready for review"

Thanks!

ssalonen commented 4 years ago

I get the below error with ExpiringStringItemIntegrationTest when running integration tests. Existing integration tests seem to pass.

Could you look into this?

com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMappingException: DynamoDBStringItem[expiration]; could not unconvert attribute
    at com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperTableModel.unconvert(DynamoDBMapperTableModel.java:271)
    at com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper.privateMarshallIntoObject(DynamoDBMapper.java:456)
    at com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper.marshallIntoObjects(DynamoDBMapper.java:484)
    at com.amazonaws.services.dynamodbv2.datamodeling.PaginatedQueryList.<init>(PaginatedQueryList.java:65)
    at com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapper.query(DynamoDBMapper.java:1506)
    at com.amazonaws.services.dynamodbv2.datamodeling.AbstractDynamoDBMapper.query(AbstractDynamoDBMapper.java:265)
    at org.openhab.persistence.dynamodb.internal.DynamoDBPersistenceService.query(DynamoDBPersistenceService.java:501)
    at org.openhab.persistence.dynamodb.internal.AbstractTwoItemIntegrationTest.testQueryUsingNameAndEnd(AbstractTwoItemIntegrationTest.java:160)
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:498)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:89)
    at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:41)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:541)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:763)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:463)
    at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:209)
Caused by: com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMappingException: could not invoke null on class org.openhab.persistence.dynamodb.internal.DynamoDBStringItem with value 1565326414 of type class java.lang.Long
    at com.amazonaws.services.dynamodbv2.datamodeling.StandardBeanProperties$MethodReflect.set(StandardBeanProperties.java:136)
    at com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperFieldModel.set(DynamoDBMapperFieldModel.java:111)
    at com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperFieldModel.unconvertAndSet(DynamoDBMapperFieldModel.java:164)
    at com.amazonaws.services.dynamodbv2.datamodeling.DynamoDBMapperTableModel.unconvert(DynamoDBMapperTableModel.java:267)
    ... 31 more
Caused by: java.lang.NullPointerException
    at com.amazonaws.services.dynamodbv2.datamodeling.StandardBeanProperties$MethodReflect.set(StandardBeanProperties.java:133)
    ... 34 more
kaikreuzer commented 4 years ago

@mindstorms6 Are you still working on this PR?

mindstorms6 commented 4 years ago

Slipped my mind - but I’ll get back on it this weekend with the requested changes. My apologies for the delay.

On Sun, Dec 8, 2019 at 1:25 PM Kai Kreuzer notifications@github.com wrote:

@mindstorms6 https://github.com/mindstorms6 Are you still working on this PR?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openhab/openhab1-addons/pull/5875?email_source=notifications&email_token=AAAWWCKLONZSCIEMOZECSZTQXVQ37A5CNFSM4H4YYNYKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGHKGSA#issuecomment-562996040, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAWWCIQDIEM4HLQ4PWJHRLQXVQ37ANCNFSM4H4YYNYA .