Skip to content

[BEAM-4571] RedisIO support for write using SET operation#6045

Merged
jbonofre merged 4 commits intoapache:masterfrom
henryken:redisio-set
Aug 13, 2018
Merged

[BEAM-4571] RedisIO support for write using SET operation#6045
jbonofre merged 4 commits intoapache:masterfrom
henryken:redisio-set

Conversation

@henryken
Copy link
Copy Markdown
Contributor

Add support for set, lpush, and rpush Redis operations, including setting expire time

R: @jbonofre

@henryken
Copy link
Copy Markdown
Contributor Author

retest this please

1 similar comment
@henryken
Copy link
Copy Markdown
Contributor Author

retest this please

@henryken
Copy link
Copy Markdown
Contributor Author

Hi, is there any comment on this pull request?

Copy link
Copy Markdown
Member

@jbonofre jbonofre left a comment

Choose a reason for hiding this comment

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

Overall looks good. Can you please just change the test methods ? I will merge just after.


@Test
public void testWriteRead() throws Exception {
public void testWriteRead_append() throws Exception {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For consistency, I would use testWriteReadAppend().

}

@Test
public void testWriteRead_set() throws Exception {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here testWriteReadSet().

@henryken
Copy link
Copy Markdown
Contributor Author

retest this please

1 similar comment
@henryken
Copy link
Copy Markdown
Contributor Author

retest this please

@henryken
Copy link
Copy Markdown
Contributor Author

Thanks for your review @jbonofre. I've made the changes and the build is good. Let me know if there is anything else.

Copy link
Copy Markdown
Member

@jbonofre jbonofre left a comment

Choose a reason for hiding this comment

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

LGTM, thanks !

@jbonofre jbonofre merged commit 9b4ed13 into apache:master Aug 13, 2018
@henryken henryken deleted the redisio-set branch May 19, 2019 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants