canonical / kafka-test-app

This is a Kafka Test app charm to be used to tests and load a Charmed Kafka Cluster
Apache License 2.0
0 stars 1 forks source link

Initial app #1

Closed welpaolo closed 1 year ago

deusebio commented 1 year ago

Generally, I would address everything related to UX and really crucial, and leave refactoring to when we will have more time. It is a fairly simple charm and refactoring won't be hard or complex. Also, since there won't be any deployment we can really change implementation anytime, as long as we keep the API reasonably stable. Actually, as mentioned in my comment, I'd really use this as toy example to also promote/play with more "advanced" features.

  1. Use idle_period= and status= in wait_for_idle checks on int-tests, much safer and stable

I agree! With the last refactoring things are way more solid, but I also have the feeling that sometimes the charm "seems" to be hanging in idle, although when an update_status comes it finalize its operations! I have also opened a PR about this for kafka-k8s operator here

  1. I don't think this charm is a great candidate for trialing the RelationDataModel stuff. It's small and minor, which is good, but it also doesn't use many complex data types, so it feels fairly bloaty here. I would rather we used that feature when it was needed, and not here

Actually since it is simple, I would really use this as a toy charm to improve the structure. As I mentioned in my comment, I'd rather leave it as is, as long as it works, since we can refactor/improve that when we will have more time

  1. Is mTLS testing part of this scope? Or is that another PR down the line?

No, not yet. Let's leave that aside for the time being.