Design process of exported APIs

[ansiwen]

Sometimes we merge changes or additions to the exported API changes "just in time" for a release, giving it not the care it should get IMHO. And I know that's also totally my fault, because I don't review in time. Given that the API is the main "feature" of go-ceph, I would like to start a discussion how we can improve the process so that we don't end up in a situation where we make compromises on the quality of the API for the sake of other priorities, or at least that when we do, we are fully aware of it. In general I don't think it's a good idea to merge something in doubt just to have "something" to release. On the other hand I also see that we (more specifically I) need a bit pressure to keep moving forward. So I will just enumerate a few things that came to my mind and we could think about:

tech preview

We could in general release all new APIs as a "tech preview" or beta or something, that similar to "deprecated" is not recommended to use yet if you require stability. I know, as long as we are below 1.0 we don't need that, but I feel in the end we almost take API stability already as serious as post-1.0. That way people who don't mind doing some fixes with the following release can use it already, and we also have a chance to get feedback on the API and improve it. Also it would be a nice escape path for feature pressure from Ceph-CSI, because we would have a way to integrate their requirements/requests, but can still change something later (because we can simply fix it on the CSI side as well by ourselves).

design spec

Without being too bureaucratic, we could establish that we first push a draft PR that contains just enough to have a conversation about the exported API. That would mean that it's expected to do all the nagging and pushing to review the API already, although implementation is not finished. ;-) After we agreed the (rest of the) implementation and its reviews follows. If we only review after basically all is done, it is psychologically quite hard to accept too much changes, because so much effort has gone into it already.

TDD (Test Driven Development)

similar to the point before, it can be quite useful to write tests already before the implementation, because it can clarify how the API is expected to be used.

feature freeze

We could say that all PRs that touch the exported API must be reviewed until 2 weeks before release, so that there is an actual chance to make changes, if required.

feature-driven releases

If, and only if, the fixed release cadence has the effect that we compromise on the quality, we might rethink it. So either we should mentally prepare that a release might sometimes have not a lot new to offer, and not feel pressure to merge something in time, or we should maybe move to a more flexible release schedule.

In the end my motivation is, that go-ceph will be better than it would be otherwise. It's not about being perfect of course, but to reach at least the 80% of the 80/20 rule. Don't let perfect be the enemy of good. But also don't let pace be the enemy of good enough.

[obnoxxx]

Can we make it so that those unstable APIs (version < 1.0) are by default "disabled", and a user has to explicitly unlock it by setting some variable / tag in the invoking code?

[ansiwen]

I can buy that, but we should have a limit on how long APIs may remain beta w/o requests for changes. For example, an API may only be beta for 2 releases and if no negative [1] feedback is provided it must be lifted to the standard api.

Of course. I didn't write that explicitly, but I presumed, that a new API would be beta just for one or two releases, and then "automatically" moves to stable, if it hasn't been touched. Of course, there shouldn't be a bigger change just a day before the release and then moved to stable, else we have the same problem again. But I assume in 99% of the cases it will just move to stable in minimal time. I like the method, because it's also a nice solution for post-1.0.

The only other concern I have with this is if there ought to be a general "beta" tag, different beta tags per-package (cephfs_beta, rbd_beta, etc), or beta tags per-feature group, something like "rbd_mirror_beta", or "cephfs_fsinterface_beta". The least granular tag is nice because its simple to use and reason about. However, it has the downside (IMO) that if you opt into one beta feature you end up opting into them all. So for example, if project x was needing a feature but we add it as beta and they enable the "beta" tag in order to get one specific thing its easier to "accidentally" use another beta feature.

Yeah, I clearly feel just one tag would be enough. We don't always protect the user from shooting in their foot, if they try hard. There are plenty of options how you could cause panics for example (which is worse IMHO, because it happens in run-time) and we have a limit of our "defensive programming", as you called it once. So, I guess it's a question where we want to set the threshold of providing hand-holding for the user. This feels like we are already clearly on the defensive side compared to our overall project average. (Because I personally would even be fine with just using comments for declaring beta APIs.)

[phlogistonjohn]

After a brief voice discussion on monday, I'm fairly strongly in favor of per-package tags name like "cephfs_alpha", "rbd_alpha", etc. This is going to protect users against accidentally opting into "alpha" features / unstable apis in subpackages that they don't need them on without being too granular. It also avoids collisions with a generic tag like "alpha" or "beta" that is not specific to go-ceph that could potentially be used by a project consuming go-ceph.

[nixpanic]

A goceph_alpha tag would be fine with me, not sure if there is a need of per package tags. I am not really opposed to per package tags, but wonder if it would be more work to maintain things.

[phlogistonjohn]

I guess "goceph_alpha" tag would solve the latter issue (of another project using an "alpha" tag). So, I won't insist on per-package tags if there isn't strong interest from the rest of the team. If we find out there's confusion around this in the future nothing stops us from using more "granular" tags when we need them.

[ansiwen]

@phlogistonjohn and me agreed out of band that both "alpha" and "beta" can create the wrong impression, so we both like goceph_preview as a tag. If there are no objections we will go with that.

[phlogistonjohn]

One other item in here that I like generally is the feature freeze idea. However, due to our traditional activity level, I suggest a one week feature freeze rather than two. However, we need to make sure active reviewer are on board with that. Especially @nixpanic and @agarwal-mudit I'll also include @leseb and @thotz who are actively working on the rgw packages. What do you think?

[ansiwen]

From chat:
We might have different understanding of the freeze. For me the more important part would be that full reviews have been done before August 3, so that there is time to implement changes. If it must be merged by that date we end up in a similar situation as before, just one week earlier. My motivation was to avoid a situation of compromised merges, because of too much time pressure between review and merge deadline.
So, I though of something like: API affecting PRs must be initially pushed at least 2 weeks before, and full reviews must be done one week before release deadline.

[phlogistonjohn]

As it's somewhat relevant to this discussion, I want to mention that I created a new "API" label. We can use this label to explicitly mark a PR as containing changes to the API. In the future we could include it as part of our automation too.
Does anyone want an explicit label for "non API" changes so we can distinguish between items that just lack a label vs. those that are already triaged as not having any api changes?

[ansiwen]

I would wait with the non-API label, if it we are really missing it.

[ansiwen]

In #552 we merged an API stability document which describes preview and deprecated APIs. So, two questions:

1. when will we start applying it? Since it is merged I would assume with the next release, right?
2. is everyone in the boat here? I assume that whole thing happened a bit under the radar of @leseb and @thotz, so a commitment from the "rgw" team would be good. ;-)

[leseb]

Works for me. Thanks for the heads up.

[phlogistonjohn]

Yeah, for rbd/rados/cephfs and the cephfs/admin pkgs we can start applying this policy now. Since rbd/admin and rgw/admin were new and already "all in" previews, albiet without any sort of end date I'm willing to be a bit squishy there. I would like those to get to this state at least by the next release, but if everyone's on board with doing that this coming release I'm OK with that too.