Skip to content

Conversation

danielwhite
Copy link
Contributor

@danielwhite danielwhite commented Feb 1, 2023

Recently, I was puzzled why github.com/gofrs/uuid was included when updating the vendored dependencies in another project. The reason for its inclusion lead to this project. As it turns out, it was introduced by #84 only to generate a ClientRequestToken for the purpose of idempotent transactions.

This proposes to generate the default token with crypto/rand.Reader instead.

This is equivalent to a Version 4 UUID except we gain an additional 6 bits of entropy. The main advantage is that this removes the only reason for importing then entire github.com/gofrs/uuid making for fewer dependencies for downstream consumers.

Without the DYNAMO_TEST_REGION environment variable set, this would
panic with the following:

	--- FAIL: TestTxRetry (0.00s)
	panic: runtime error: invalid memory address or nil pointer dereference [recovered]
		panic: runtime error: invalid memory address or nil pointer dereference
	[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1594100]

	goroutine 57 [running]:
	testing.tRunner.func1.2({0x164e340, 0x1d420f0})
		testing/testing.go:1396 +0x24e
	testing.tRunner.func1()
		testing/testing.go:1399 +0x39f
	panic({0x164e340, 0x1d420f0})
		runtime/panic.go:884 +0x212
	github.com/guregu/dynamo.(*Put).run.func1()
		github.com/guregu/dynamo/put.go:96 +0x20
	github.com/guregu/dynamo.retry({0x198af68, 0xc00008e060}, 0xc00012dd40)
		github.com/guregu/dynamo/retry.go:33 +0xb4
	github.com/guregu/dynamo.(*Put).run(0xc0000c00e0, {0x198af68, 0xc00008e060})
		github.com/guregu/dynamo/put.go:95 +0x1ef
	github.com/guregu/dynamo.(*Put).RunWithContext(...)
		github.com/guregu/dynamo/put.go:63
	github.com/guregu/dynamo.(*Put).Run(0xc0000c00e0)
		github.com/guregu/dynamo/put.go:57 +0x8d
	github.com/guregu/dynamo.TestTxRetry(0xc000082d00)
		github.com/guregu/dynamo/tx_test.go:186 +0x245
	testing.tRunner(0xc000082d00, 0x1733f48)
		testing/testing.go:1446 +0x10b
	created by testing.(*T).Run
		testing/testing.go:1493 +0x35f
	exit status 2
This is equivalent to a Version 4 UUID except we gain an additional 6
bits of entropy. The main advantage is that this removes the only
reason for importing then entire `github.com/gofrs/uuid` making for
fewer dependencies for downstream consumers.
@guregu
Copy link
Owner

guregu commented Feb 1, 2023

Sounds good to me, less dependencies is always better 👍

@guregu guregu merged commit 47a9c7a into guregu:master Feb 1, 2023
@guregu
Copy link
Owner

guregu commented Feb 1, 2023

Just released in v1.18.1, thanks!

@danielwhite danielwhite deleted the remove-gofrs-uuid-dependency branch February 1, 2023 23:03
@danielwhite
Copy link
Contributor Author

Thanks for the quick turnaround!

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