Breaking changes for v2.0

[guregu]

Eventually I'd like to put out a v2.0 and improve some things. Feel free to comment with changes you'd like to see made.

Here's some stuff I'd like to examine:

• The current BatchGet and BatchWrite API is kind of awkward. We should optimize it for some common use cases. Maybe they should work more like the transactions API. Maybe there should be 2 different kinds of batch APIs, one for multi-table access like transactions and one for single table. Definitely want to get rid of dynamo.Keyed or make it more useful somehow.
• The ConsumedCapacity struct was made before read/write unit data was part of the API. We should make it easy to access that in addition to the total, perhaps as a separate Capacity struct for indexes (Can't differentiate between read and write in ConsumedCapacity #112). Maybe use a different kind of ConsumedCapacity for transactions?
• Better integration with struct tags (so you don't have to specify the key names every time)? Or maybe a higher-level 'schema' package?

[guregu]

It's probably a good idea to re-examine the method chaining style in general. Should we use interfaces instead of structs (#143)? Should we switch to functional parameters? Should we wait until Go 2 and use generics somehow? Input is welcome.

Method chaining structs (current style)

• Easy to use
• Hard to mock
• Not idiomatic?

Method chaining + interfaces

• Easier to mock
• Godoc slightly worse?
• Harder to copy (clone) queries? They're probably not fully clone-able now as structs (inner pointers, maps etc).
  • Could be solved with a Clone() method.

Functional parameters

• Trendy
• Idiomatic?
• Possibly hard to discern which options are for which thing? Needs some experimentation.

Go 2 (generics)

• No guarantee when it's released
• Needs experimentation, could potentially be cool. Might solve the functional param issues.

[Deewai]

Hi @guregu, mocking will become easy when returning interfaces, tho, it's generally advisable to receive interfaces return concrete types.
I also had issues with mocking the methods and my solution was a little messy. I basically had to also go against the rule of receiving interfaces and returning concrete types.
On the other hand using functional parameters will help with receiving interfaces and this can be mocked, but like you said one who is not really familiar with the code base wouldn't know which methods are available to use for certain things.

In my opinion i'll go in favour of method chaining + interfaces

[mingrammer]

Will dynamo v2 also support aws-sdk-go v2?

[guregu]

@mingrammer I think so, yes. It's a good opportunity to make breaking changes, so I want to switch to aws-sdk-v2 if possible.

[SubNader]

Hello @guregu, are there plans to switch to/support AWS SDK V2 anytime soon?

[guregu]

@SubNader My current plan is to let the dust settle with Go 1.18 and generics and see where it goes from there. I would like to create an alpha version of v2 after Go 1.18 so we can play around with improving the API.

Ideally I would like v1 to be "feature complete" before we start on v2 because I don't want to maintain two versions but we'll see how it goes 👀. There are not too many features I still want to add, but it might be a good idea to add missing things like PartiQL before we make a new version.

[abrekhov]

Waiting so much for v2 support!

[irohiroki]

I started using dynamo and like to have AWS SDK v2 support. As @guregu seems busy, I think I would give a try to make a PR that supports sdk v2.

I don't know if sdk v2 support and the interface changes discussed above should be out at the same time, but anyway IMHO sdk v2 support PR could be of some help in some way.

[guregu]

@irohiroki Yes, that would be helpful. I can use it as a base for the next version. This fork also seems to do the same thing but I have not looked at it closely: https://github.com/niltonkummer/dynamo

[guregu]

I see there's a lot of demand for AWS SDK v2 support. Maybe we should consider a minimal v2 and release it sooner rather than later.

It might be easier to think of things that we need to add to v1 before v2. I'd rather not maintain two branches at once, so v1 should be 'done' by the time v2 is out.

Things we probably need to do in v1

• Parallel scan
• Refactor/optimize marshaling?
• Maybe add support for the rest of the APIs we are missing? (PartiQL, backups, ...)

Things I definitely want for v2

• AWS SDK v2
• All methods take context.Context
• Refactor batch APIs
  • Rework BatchWrite/BatchGet to be more like the transactions API (support multiple tables)
  • Get rid of Keys/Keyed
  • No more weird .Batch() accessor

Nice to have

• Some way to define schemas/facets/something so that you don't have to type the attribute names for PKs each time

[irohiroki]

I just made #206 Migrate to AWS SDK v2 🚀

[tamccall]

On the topic of making breaking changes to the BatchGet / BatchWrite APIs i think that it is a bit awkward that the Batch() method is a receiver on the table struct.

dynamo/batchget.go

Lines 18 to 43 in a749b0c

	// Batch stores the names of the hash key and range key
	// for creating new batches.
	type Batch struct {
	table Table
	hashKey, rangeKey string
	err error
	}
	// Batch creates a new batch with the given hash key name, and range key name if provided.
	// For purely Put batches, neither is necessary.
	func (table Table) Batch(hashAndRangeKeyName ...string) Batch {
	b := Batch{
	table: table,
	}
	switch len(hashAndRangeKeyName) {
	case 0:
	case 1:
	b.hashKey = hashAndRangeKeyName[0]
	case 2:
	b.hashKey = hashAndRangeKeyName[0]
	b.rangeKey = hashAndRangeKeyName[1]
	default:
	b.err = fmt.Errorf("dynamo: batch: you may only provide the name of a range key and hash key. too many keys")
	}
	return b
	}

Seems like this should be moved to be a reciever on the DB struct and then somehow through interactions with the BatchGet / BatchWrite clients would specify the table similar to how the GetTx and WriteTX work.

I could believe that some version of this idea could be warranted

Maybe there should be 2 different kinds of batch APIs, one for multi-table access like transactions and one for single table

but just wanted to say that in using this new branch I don't the current apis particularly intuitive for doing multi-table reads.

[guregu]

Yeah, the current Batch APIs are my biggest regret for this library. The transaction APIs are the atonement for my sins (they are closer to how the batch APIs ought to look, probably). The recent additions of multi-table stuff is mostly just trying to hack it in without breaking the API. Removing the current batch APIs for v2 will probably break too much existing code for an easy transition but a separate API based on DB methods would be a nice addition I think.

[tamccall]

Removing the current batch APIs for v2 will probably break too much existing code for an easy transition

I mean updating to use v2 will require code updates to adopt so perhaps this is the time to address in spite of that.

That said if you're looking for some half-way solutions perhaps you could add a new api that is more in line with the vision and mark the old API as // Deprecated: .