Command Migration: ('HINCRBY', 'HINCRBYFLOAT', 'HRANDFIELD')

[sashpawar11]

This PR migrates HINCRBY, HINCRBYFLOAT and HRANDFIELD to make it protocol agnostic. Closes #1023

Migration Checklist

• Migrated the evalXXX function with the latest definition
• Update or add unit tests for the new implementation.
• All unit tests pass successfully.
• Ensure all integration tests pass successfully.
• Ensure integration tests are added for the migrated command on multi-threaded resp server.
• Add http, websocket tests for the commands
• Move Integration tests for the respective commands under the RESP integration tests directory from Async directory
• Please validate that the documentation for the respective commands is up to date. If not then consider adding them.

Note:

• Http, Resp and Websocket tests to be added as well since they are missing for the addressed commands under this issue.
• Docs for these commands have been added

[sashpawar11]

Hi @AshwinKul28
update 10/15 - unit tests have been migrated, please have a look and provide feedback! The unit tests have passed succesfully.

Will continue to work on migrating the integration tests.
Thankyou.

[sashpawar11]

@AshwinKul28 - the HRANDFIELD command depends on selectRandomFields() for returning the random field.
Should we move it to hash.go which feels more relevant as it has other utility methods around hashmaps?
Currently, selectRandomFields()has been migrated to store_eval.go from eval.go under this PR

[soumya-codes]

@sashpawar11 lets ensure that the corresponding integration tests are migrated as well. cc @AshwinKul28

[sashpawar11]

Hi @AshwinKul28 - for the last task, I'm not sure how to proceed on it.
Are these commands optimized for multi-threaded execution yet?

The unit tests and integration tests have passed in the latest build

[soumya-codes]

@sashpawar11 please refer to existing integration tests under integration_tests/commands/resp for the commands that are already migrated.
We will need to do the same these commands as well.

[sashpawar11]

@soumya-codes Awesome, thanks!! Will have a look at it and migrate these commands.
Thankyou

[sashpawar11]

Update 10/16 - added migrated resp tests for the commands. All the tasks have been completed and build passed, however, the http and websocket integration tests are not present for these commands. Added an additional task in the description for the same, will complete it by today/tomorrow and update here once ready for review.

cc @AshwinKul28 @soumya-codes

[sashpawar11]

Update 10/18 - Added all the missing HTTP integrations tests for the command. Will try to complete the web-socket tests asap and mark this PR for review.
cc @AshwinKul28 @soumya-codes

[sashpawar11]

Hi @AshwinKul28 @soumya-codes @lucifercr07 - all the commands have been migrated and their respective tests have been added. Resolved multiple merge conflicts along the way.

The PR Is ready for review.

The documentation for the commands are missing and I'll try to add it via a separate PR as this one is already a bit large.
Thankyou.

[sashpawar11]

@AshwinKul28 - the HRANDFIELD command depends on selectRandomFields() for returning the random field. Should we move it to hash.go which feels more relevant as it has other utility methods around hashmaps? Currently, selectRandomFields()has been migrated to store_eval.go from eval.go under this PR

Also, please advise on this.
cc @AshwinKul28 @soumya-codes @lucifercr07

[lucifercr07]

Can we also check the doc related to 'HINCRBY', 'HINCRBYFLOAT', 'HRANDFIELD' commands also once, if there's any discrepancy?

Minor comments added.

[sashpawar11]

Can we also check the doc related to 'HINCRBY', 'HINCRBYFLOAT', 'HRANDFIELD' commands also once, if there's any discrepancy?

Minor comments added.

Hey @lucifercr07 - there no docs present for these commands at the moment. Will be adding them in a seperate pr as I complete working on it.

[sashpawar11]

Hi @AshwinKul28 @soumya-codes @lucifercr07 - all the tasks have been completed. Please review the changes!

Latest changes:

• Documentation for the commands have been added as well.
• selectrandomfields() has been moved over to hmap.go

[AshwinKul28]

Hey @sashpawar11 this is a commendable effort. Everything looks great to me. Few cosmetic changes are required. (I know this is a little back and forth, apologies for that. But we want to make it a one-time effort, and some ideal PRs to merge)

[sashpawar11]

Thanks @AshwinKul28 ! And yes I agree, lets complete all the changes to make this a one-time effort

[sashpawar11]

Conflicts resolved. Ready for merge.
Thankyou