Skip to content

Conversation

bringyou
Copy link

@bringyou bringyou commented Mar 8, 2018

JedisConnection's constructor(line 125) calls a public method, the select(dbIndex). Method call inside constructor can be dangerous, because a bad subclass may hurt super class by overriding that method. And consider that when we construct a JedisConnection, it won't be in transaction or piplined, so change to use jedis.select directly

@pivotal-issuemaster
Copy link

@bringyou Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@bringyou Thank you for signing the Contributor License Agreement!

@mp911de
Copy link
Member

mp911de commented Mar 8, 2018

Care to file a Jira ticket first before proposing code changes?

@bringyou
Copy link
Author

bringyou commented Mar 9, 2018

sorry, I created a jira before this pr but I forget to post here. This is the url
https://jira.spring.io/browse/DATAREDIS-781

@bringyou
Copy link
Author

bringyou commented Mar 9, 2018

And close() is called too, maybe should be changed together, but I didn't have any ideas on how to change.😫

@mp911de mp911de changed the title change select() to jedis.select in JedisConnection's constructor DATAREDIS-781 - Change select() to jedis.select() in JedisConnection's constructor Mar 14, 2018
@mp911de
Copy link
Member

mp911de commented Mar 14, 2018

Thanks a lot. I left a comment in the related ticket.

@mp911de
Copy link
Member

mp911de commented Mar 21, 2018

Closing without merge as per comment in the associated ticket.

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.

3 participants