CASSGO-29, WIP refactor packages to separate frontend api to backend internal api.

[OleksiienkoMykyta]

No description provided.

[OleksiienkoMykyta]

@joao-r-reis here is one more WIP implementation for (#465), focused on solving issues you mentioned in previous PR(#1850).
A lot of work is left, but I would appreciate your opinion about the direction. If it's any good, I will keep working on this implementation.

[joao-r-reis]

You still have a public protocol package there. Let's just move everything that is private to the internal package and everything else stays in the main gocql package as it exists today.

So 1 single internal package that contains stuff that is not exported publicly by gocql's interface today and everything else keep unchanged in the gocql package.

After this we can start thinking about potentially a better organization for the code under internal but let's not worry about that yet. The most useful and complex part is splitting the code between these 2 places: "internal" (single internal package for now) and "public API" (gocql package)

[OleksiienkoMykyta]

You still have a public protocol package there. Let's just move everything that is private to the internal package and everything else stays in the main gocql package as it exists today.

So 1 single internal package that contains stuff that is not exported publicly by gocql's interface today and everything else keep unchanged in the gocql package.

After this we can start thinking about potentially a better organization for the code under internal but let's not worry about that yet. The most useful and complex part is splitting the code between these 2 places: "internal" (single internal package for now) and "public API" (gocql package)

Yes, I have created the protocol package to have an opportunity to use types and functions used in the gocql and the internal packages, to avoid cycle imports, the internal doesn't have separation on sub-packages. The idea is to move everything that should be moved to a single internal package and save backward compatibility. So the public API types and functions are just duplicated to separate packages, and marked as deprecated in the gocql.
I tried to gracefully separate the internal and public API and avoid cycle importing and breaking backward compatibility.
So, if I get it in the right way, do you want me to move only functionality that can be moved without any changes to public API (even if it's provided with backward compatibility)?
For example:

func encVint(v int64) []byte {
	vEnc := encIntZigZag(v)
	lead0 := bits.LeadingZeros64(vEnc)
	numBytes := (639 - lead0*9) >> 6

	// It can be 1 or 0 is v ==0
	if numBytes <= 1 {
		return []byte{byte(vEnc)}
	}
	extraBytes := numBytes - 1
	var buf = make([]byte, numBytes)
	for i := extraBytes; i >= 0; i-- {
		buf[i] = byte(vEnc)
		vEnc >>= 8
	}
	buf[0] |= byte(^(0xff >> uint(extraBytes)))
	return buf
}

Because I thought that the aim was to completely separate the public and internal API's.
Thank you for the detailed review, I appreciate it!

[joao-r-reis]

I'm a bit confused about the encVint example, that's a private function that should be easily moved to the internal package without much change right?

Hmm moving every single type and function that is private to internal is probably not the best idea because it will create a lot of cycle importing as you mention, I realize this now. Maybe we should only try to move code to internal that is a bit more isolated.

The original GH issue mentions the marshaling code which should be easy to move (marshaling related functions should be pretty much self contained). It will still require some duplication (Marshaler interface probably?) but it should be minimal.

Moving the eventDebouncer should also be quite easy to do.

Code in frame.go is a bit more tricky, there's a lot of the protocol stuff that has to be part of the public API so moving it would result in a decent amount of duplicated code. To remove this duplication I think it would require some degree of refactoring so I think we can just leave it in the public gocql module for now?

Code in token.go seems like it can be moved pretty easily. Same with the prepared cache. Same with code in ring.go.

Conceptually code related to connections, control connection, connection pools, etc. should also be able to be moved to an internal package but it might require some refactoring to limit the amount of duplication... Control connection and connection pool depends on the Conn type so any effort here would start with the Conn type which also seems to be the most complicated one to move because it actually has some public API stuff in there. It's probably best to keep all of this connection related code in the gocql module for now. It might be easy to move the writeCoalescer type though.

There's parts of metadata.go that could maybe be moved to an internal package but this one also seems a bit tricky, there's some public API dependencies here.

logger.go should be easily moved to internal but the interface probably needs to be duplicated.

It looks like topology.go code can also be moved easily? Not 100% here.

In summary we should go on a case by case basis and if we see a lot of public API dependencies on a particular part of the code then we should probably leave it on the gocql package for now and focus on parts of the code that don't have those dependencies.

[OleksiienkoMykyta]

I'm a bit confused about the encVint example, that's a private function that should be easily moved to the internal package without much change right?

Hmm moving every single type and function that is private to internal is probably not the best idea because it will create a lot of cycle importing as you mention, I realize this now. Maybe we should only try to move code to internal that is a bit more isolated.

The original GH issue mentions the marshaling code which should be easy to move (marshaling related functions should be pretty much self contained). It will still require some duplication (Marshaler interface probably?) but it should be minimal.

Moving the eventDebouncer should also be quite easy to do.

Code in frame.go is a bit more tricky, there's a lot of the protocol stuff that has to be part of the public API so moving it would result in a decent amount of duplicated code. To remove this duplication I think it would require some degree of refactoring so I think we can just leave it in the public gocql module for now?

Code in token.go seems like it can be moved pretty easily. Same with the prepared cache. Same with code in ring.go.

Conceptually code related to connections, control connection, connection pools, etc. should also be able to be moved to an internal package but it might require some refactoring to limit the amount of duplication... Control connection and connection pool depends on the Conn type so any effort here would start with the Conn type which also seems to be the most complicated one to move because it actually has some public API stuff in there. It's probably best to keep all of this connection related code in the gocql module for now. It might be easy to move the writeCoalescer type though.

There's parts of metadata.go that could maybe be moved to an internal package but this one also seems a bit tricky, there's some public API dependencies here.

logger.go should be easily moved to internal but the interface probably needs to be duplicated.

It looks like topology.go code can also be moved easily? Not 100% here.

In summary we should go on a case by case basis and if we see a lot of public API dependencies on a particular part of the code then we should probably leave it on the gocql package for now and focus on parts of the code that don't have those dependencies.

encVint is an example of code that is easy to move, it was a little bit unclear.
Ok, now the approach is much clearer, I think it will be easier to do this way. The last question is should we keep this implementation in this brunch, or just update this one? It could be useful for future implementation if we are going to make such changes.
Thank you for the review, you helped me a lot!
Merry Xmas and have a great holidays!)

[joao-r-reis]

Ok, now the approach is much clearer, I think it will be easier to do this way. The last question is should we keep this implementation in this brunch, or just update this one? It could be useful for future implementation if we are going to make such changes.

I don't mind either way, your choice 👍

Merry Xmas and have a great holidays!

You too!