-
Notifications
You must be signed in to change notification settings - Fork 247
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat_: use content-topic override for all community filters #5993
base: feat/comm-content-topic-poc
Are you sure you want to change the base?
feat_: use content-topic override for all community filters #5993
Conversation
Jenkins BuildsClick to see older builds (128)
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/comm-content-topic-poc #5993 +/- ##
===============================================================
- Coverage 61.40% 61.24% -0.17%
===============================================================
Files 833 833
Lines 109934 109885 -49
===============================================================
- Hits 67506 67294 -212
- Misses 34558 34678 +120
- Partials 7870 7913 +43
Flags with carried forward coverage won't be shown. Click here to find out more.
|
f028f84
to
cb59e06
Compare
12451a8
to
d61d200
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chaitanyaprem can you please rebase, the diff seems wrong, thanks 🙏
cb59e06
to
54b1cd7
Compare
b5315d5
to
103b415
Compare
b4b929f
to
e8877b1
Compare
6219ad7
to
c0ccc28
Compare
eedb0da
to
e49e585
Compare
c0ccc28
to
00ea105
Compare
00ea105
to
eba057f
Compare
b247503
to
654cd34
Compare
654cd34
to
cbaee58
Compare
cbaee58
to
60a8ed8
Compare
342a0da
to
ae2cada
Compare
60a8ed8
to
0073156
Compare
I have done basic testing of this PR locally. This needs to be QA tested. Not sure what is the release timeline this has to be included in. Since this breaks compatibility with any code before #5864 (probably planned for 2.33), i will leave it upto desktop and mobile teams to decide when to include this. As community chats will not interop with any code before #5864 . @iurimatias and @ilmotta |
Perhaps more than when to include the changes from this PR (2.33, 2.34, etc), I'm thinking about how to include these changes in a way that minimizes impact to users of Status. @fryorcraken: in the forum you asked about the deprecation policy for Status. I've never heard of such a policy. In a discussion not so long ago (can't remember if it was about single content topic), I remember @osmaczko suggested the Status app could deprecate XYZ based on % of usage, not on arbitrary release versions/steps. This gradual rollout strategy is what I tend to prefer if feasible so that users can plan when they upgrade, not us CCs. Up to a point of course because at a certain low threshold of usage the maintenance cost of multiple implementations becomes hard to justify. @chaitanyaprem in this forum post you shared a potential solution where a message in the community could be broadcast, asking users to upgrade. Another person suggests We can then warn users who still publish to old content topics for several versions before the compatibility break is effected. Both ideas seem simple and could work well 👍🏼 @osmaczko @iurimatias what do you think should be the migration plan for introducing the changes from phase 2 in Status? |
How would you determine the current % usage? How are we supposed to improved protocol behaviour, ie, reliability and scalability, if we are enable to properly plan breaking change upgrades?
All major software have a way to deprecate old versions. https://nodejs.org/en/about/previous-releases: unsupported and LTS For centralized mobile app, the app will often force you to upgrade to continue using. We are not talking about something like that, but we are talking about breaking compatibility between 2 specific versions. Users can keep the old versions, as long as their friends and community do, |
@fryorcraken The first step is to determine whether gradual phasing out or rollout is something we want in Status. I haven't thought about the how, and I'm probably not the best person to answer this question in detail either. For mobile apps, most Android users are automatically tracked by Google, and we can view the number of devices using any specific Status version as a time series in the developer portal. Since Android users dominate most of Status's metrics, this data can serve as a strong indicator for deciding when to roll out breaking changes. For Desktop users, it's more challenging (for good reasons) as there's no entity automatically tracking usage. For iOS, approximately 25% of users of Status allow Apple to collect usage data, such as the app version. Additionally, some users opt in to help Status with analytics, which provides further insights into the usage of different versions. Another way to understand usage is by leveraging the Status proxy. The proxy is a centralized piece of software, like many others, and we could use it to track the app version to precisely identify which versions are in use. I say precisely because, unfortunately, the proxy is currently mandatory. Hopefully, in the not so distant future, we'll give users the option to disable the proxy and choose their own RPCs, similar to what Metamask allows. Therefore, as we can see, there are ways at the Status app level to analyze and track usage by version over time with a decent degree of certainty. To clarify, by
I was referring to your question in the forum @fryorcraken The gradual rollout is my preferred option when available and when meeting business and development criteria. If we want to urgently introduce breakage, then it may not be the best solution.
I'm not sure I understand this question. Gradual rollouts allow changes in the protocol as well, at the expense of (maybe) added complexity and time, but more respectful to users. There are tradeoffs, but it's not an impediment to improvement. In Status mobile we know for sure the majority auto-upgrade the app. This means we might not have to wait too much to more safely introduce breakage. I would be fine introducing breakage in 2.33 if the majority wants to go in that direction. Seems like an interesting topic for one of our weekly calls or sync calls between teams (and including the @status-im/status-go-guild). |
Indeed, the question how to handle breaking upgrades in the forum has remained unanswered. Thank you for re-opening the discussion in the context of this PR.
I agree with gradual roll outs, as described in the forum post you quoted. What I am worried about, is undefined timelines. Because this not only create complexity, debt, and makes roadmap/release planning harder. But it also makes managing user expectations more difficult. Having a guideline such as: Makes it clear that:
In the context of this change, moving communities to a single content topic has 3 main benefits:
The first phase is implements sending messages on a unique content topic. But we keep expecting messages on all content topics. Which means none of those benefits are delivered at present. Only once we can break backward compatibility can the users benefit of this change. Waiting for (let's say) at least 90% of users to upgrade to a version that has phase 1 before we go for phase 2 means:
Finally, as you mentioned on android you have an auto-upgrade path. Which means that android usage numbers are likely to be more optimistic that desktop numbers. So we might suprise some desktop users that do not pay attention because a specific version number breaks compatibility. A consistent messages is more likely to keen users better informed. |
Thanks @fryorcraken, you cleared up some things for me.
My prior understanding was that we could support both solutions and still benefit the network because the majority would likely upgrade within a reasonable timeframe, and therefore we would be able to wait for at least a few months and likely break less users. If we can only realize the benefits all at once then there's little benefit to break based on usage.
I agree planning becomes less predictable, but the benefit is supposedly for users, not us. Breaking only with hard deadlines or hard release numbers is unquestionably simpler and cheaper for us. A gradual roll out does not mean deadlines can't be established. There are different ways to define the threshold to break. We can roll out gradually but establish a hard 6 month break period. Meaning, Status will break as fast as possible based on usage at X%, and definitely break in 6 months or any other period we think is reasonable for the breakage in question. This is more flexible than only the deadline/version model, albeit with added planning complexity as you say.
Depends on the user's preference a bit. A user may be fine with a slightly more convoluted explanation, in exchange for more time to adjust. A more technical user may even like this strategy because it may imply the software breaks closer to the theoretical best. But of course I don't know what the majority of users of Status prefer. That's one reason I lean on the side of caution when I read suggestions to break at Status version X without evidence to support the choice except our own cost and product requirements (which to be fair are plenty important).
Indeed, this would always be a risk. Maybe too much of a risk if the breakage is introduced too fast based on Mobile metrics. To some extent, we can make good guesses about Desktop versions because some users do enable analytics and we have the Status proxy to cross-reference data and guess opt-in rate. Even if we make the proxy optional, many users would likely enable the proxy in order to have a good out of the box experience, especially for the wallet features. Having said all that @fryorcraken, I agree with you on your recommendation to go for a simpler approach for phase 2. I would still like to hear other folks' opinions about when to best merge this PR into mainline cc @iurimatias. If that's 2.33 then we and Comms should probably start to communicate in January because 2.33 may be released still in February and we want users to have time to plan their upgrades. Might be better to go for 2.34 instead. |
I believe @Samyoul provided insight into how it has been done so far, but under a different post: https://forum.vac.dev/t/breaking-changes-and-roll-out-strategies/338/3.
In the context of this breaking change, I believe it is technically feasible, though not entirely straightforward. We could monitor all public channel communication propagated through waku and aggregate it by user (public key) and content topic (whether legacy or new). However, this would only provide an estimate, as it would not account for users communicating exclusively in private channels.
+1 I'll try to formalize it below.
where y>x. Switch to Switch to Proposed parameters: I added another condition: the version gap threshold. I’m not sure if this is something we want. It would allow us to bypass the |
cc> +1 I'll try to formalize it below.
Thanks @osmaczko, looks reasonable. I suggest for Status lead to align on this and provide an official guideline/response. The forum thread you mentioned would be a good spot. cc @sunleos |
@fryorcraken @sunleos We have two definitions to eventually align on, one is about how Waku breaking changes should be introduced into Status, and the other is about how Status handles deprecation of anything. Sometimes the line may be blurry because general recommendations may encompass Waku breaking changes, but not necessarily the opposite.
👍🏼 @osmaczko The combination of other application-level metrics alongside data from Waku will allow us to minimize the impact on users. The app-level metrics are readily available already. Since we are going for a more precise definition with a formula, we may as well include in the usage percentage a margin for error, and the margin would depend on the data source. Sometimes, for instance, if the breaking change isn't related to Waku and only affects iOS users, and we know only 25% of the user base opted in App Store Connect, we would need to be more careful with the parameters. |
So we are clear, there are no breaking changes coming from Waku at any time soon. |
Yes, we are clear @fryorcraken. The conversation was never about Waku protocols since we are talking about community-level changes, but from now on I'll try to be more specific and avoid generically saying "Waku". Re-reading now I see how that can be confusing to you and other Waku CCs. |
This is second phase of content-topic changes for communities where single content-topic is used for sending as well as receiving. This is follow-up change of #5864 and should be released only once all users have migrated to a version using #5864 as this change breaks compatibility for communities and users on any version before #5864.
Important changes:
If possible , release phase-2 as well along with phase-1 changes for new communities that would be created . But rollout 2 phases separately for existing communities as being discussed here [Deliverable] Review usage of content topics in Status Communities protocol waku-org/pm#268 (comment). @osmaczko would it be as simple as adding acommunityVersion
field in communityDescription? Wondering if all existing communities can be labelled as version 1 and communities created after this change be version 2 and use the version to determine how content-topics shall be used.