Adding batch wrapper on query builder

[mariosttass]

trafficstars

Hello,

In the following PR we are adding a Batch wrapper on gocqlx library for binding the object instead of using named columns.

Please have a look and let me know for any questions.

Thanks !

[tehsphinx]

In case you are not interested in adding this to the library, let us know. Then we'll keep it internally. (which would be harder because of missing access to private gocqlx functionality)

[tehsphinx]

@mmatczuk: Any update for us? What do you think about this MR?

[tehsphinx]

@tzach can you have a look? (this is the PR I mentioned on P99 Conference)

[mmatczuk]

Can you squash your commits to one and add a test please.

[tehsphinx]

Thx for taking a look @mmatczuk!

Can you give us a pointer on how to test this?

We had some tests but they basically mocked everything away not really testing anything in the end. So I removed them. Also note, that we are only using existing functionality that has been tested already. gocql.Batch (incl. *batch.Query method) is tested in the underlying repository, Queryx and bindStructArgs is tested in query_test.go.

[tehsphinx]

Hey @mmatczuk, we have rebased and squashed the commits 🙂

[mmatczuk]

Ok would be nice to add some docs so that the users know what it does and why. I'd ask again to add that to a test. Feel free to copy existing test and replace batch handling with your code.

[tehsphinx]

@mmatczuk Sry for the wait. I added some tests now. Also realized the execution functions for batching needed an overwrite to complete the user experience.

Let me know if there is anything else to change 🙂

[tehsphinx]

@tzach: can you plan this in to be checked?

[tzach]

@avelanarius can you please advise?

[tehsphinx]

@tzach Happy new year 🥳!

Can I draw your attention to this once more? Currently we are using this change with a go.mod replace statement. That is not ideal especially since more one more of our microservices need to do that replace.

I'll give this another 2 to 3 weeks and then I'm going to use our clone permanently, changing it's project path so we can import it from our clone without a replace statement.

[tehsphinx]

@tzach: on the last scylla conference you said, that PRs are always welcome. Seeing all the PRs here waiting for months without any attention paints a very different picture. 😞 😞

[mykaul]

@tzach: on the last scylla conference you said, that PRs are always welcome. Seeing all the PRs here waiting for months without any attention paints a very different picture. disappointed disappointed

You are right and we should strive to do a better job here - we are slowly going over the backlog and trying to get stuff in. We wish to keep high quality bar so trying not to blindly merge stuff without fully reviewing the changes. That takes time and resources.

[zimnx]

Please squash commits into single one with descriptive commit message. Thanks

[tehsphinx]

@zimnx thx for the review. I applied the requested changes :)

[tehsphinx]

Note: adding t.Parallel() to the sub-test in batchx_test.go will close the session before the sub-test is done (defer session.Close() executes too early).

EDIT: adjusted to use t.Cleanup instead of defer.

[tehsphinx]

@zimnx Thanks! Adjusted 😄