Skip to content
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

fix: reporting connection change to filter manager #5675

Merged
merged 2 commits into from
Aug 9, 2024

Conversation

chaitanyaprem
Copy link
Contributor

@chaitanyaprem chaitanyaprem commented Aug 8, 2024

While working on #5653, had noticed an issue in connection change notification given to filterManager.

The notification is being sent only for defaultShardPubSubTopic which means that any queued filters on the shard 64 will not get registered when they were adding in case user was in offline status.
This is a corner scenario that can happen during startup or when user looses connection, but nonetheless effects user experience in lightMode.

Important changes:

@status-im-auto
Copy link
Member

status-im-auto commented Aug 8, 2024

Jenkins Builds

Click to see older builds (17)
Commit #️⃣ Finished (UTC) Duration Platform Result
✖️ 2f28cf8 #1 2024-08-08 07:25:50 ~1 min tests 📄log
✔️ 2f28cf8 #1 2024-08-08 07:27:05 ~2 min tests-rpc 📄log
✔️ 2f28cf8 #1 2024-08-08 07:28:40 ~4 min linux 📦zip
✔️ 2f28cf8 #1 2024-08-08 07:30:05 ~5 min android 📦aar
✔️ 2f28cf8 #1 2024-08-08 07:33:41 ~9 min ios 📦zip
✖️ d7023e9 #2 2024-08-08 07:33:29 ~58 sec tests 📄log
✔️ d7023e9 #2 2024-08-08 07:33:37 ~1 min tests-rpc 📄log
✔️ d7023e9 #2 2024-08-08 07:34:01 ~1 min android 📦aar
✔️ d7023e9 #2 2024-08-08 07:34:23 ~1 min linux 📦zip
✖️ d7023e9 #3 2024-08-08 07:37:18 ~1 min tests 📄log
✔️ d7023e9 #2 2024-08-08 07:41:50 ~6 min ios 📦zip
✖️ d7023e9 #4 2024-08-08 07:56:24 ~1 min tests 📄log
✖️ bb944f8 #5 2024-08-08 07:59:48 ~53 sec tests 📄log
✔️ bb944f8 #3 2024-08-08 08:00:31 ~1 min android 📦aar
✔️ bb944f8 #3 2024-08-08 08:01:12 ~2 min tests-rpc 📄log
✔️ bb944f8 #3 2024-08-08 08:02:42 ~3 min linux 📦zip
✔️ bb944f8 #3 2024-08-08 08:03:22 ~4 min ios 📦zip
Commit #️⃣ Finished (UTC) Duration Platform Result
✖️ 00d0661 #6 2024-08-08 11:32:39 ~1 min tests 📄log
✔️ 00d0661 #4 2024-08-08 11:32:42 ~1 min android 📦aar
✔️ 00d0661 #4 2024-08-08 11:33:20 ~1 min linux 📦zip
✔️ 00d0661 #4 2024-08-08 11:33:21 ~2 min tests-rpc 📄log
✔️ 00d0661 #4 2024-08-08 11:35:28 ~4 min ios 📦zip
✖️ 00d0661 #7 2024-08-08 12:36:47 ~1 min tests 📄log
✔️ 6f9d87a #5 2024-08-08 13:03:20 ~2 min android 📦aar
✔️ 6f9d87a #5 2024-08-08 13:03:32 ~2 min linux 📦zip
✔️ 6f9d87a #5 2024-08-08 13:03:33 ~2 min tests-rpc 📄log
✔️ 6f9d87a #5 2024-08-08 13:05:34 ~4 min ios 📦zip
✔️ 6f9d87a #8 2024-08-08 13:46:27 ~45 min tests 📄log

@chaitanyaprem chaitanyaprem force-pushed the fix/filter-connection-change branch 2 times, most recently from d7023e9 to bb944f8 Compare August 8, 2024 07:58
@chaitanyaprem
Copy link
Contributor Author

@qfrank tests wont run because of commit convention not being followed in one of the older commits.
can you fix that, otherwise no PR's can be tested and merged into release branch.

@qfrank
Copy link
Contributor

qfrank commented Aug 8, 2024

@qfrank tests wont run because of commit convention not being followed in one of the older commits. can you fix that, otherwise no PR's can be tested and merged into release branch.

I don't have the authority, but @siddarthkay can do me a favor, let me DM him, sorry for the inconvenient

@siddarthkay
Copy link
Contributor

My bad, I've fixed history.

@siddarthkay
Copy link
Contributor

@chaitanyaprem : you'll still have to rebase

@chaitanyaprem
Copy link
Contributor Author

Looks like i don't have permissions to merge this. Maybe @richard-ramos , you can?

@richard-ramos
Copy link
Member

cc: @igor-sirotin

@siddarthkay
Copy link
Contributor

Should this go through some QA on mobile side before we merge this?
cc @richard-ramos @cammellos

@siddarthkay
Copy link
Contributor

Also ideally we should land things in develop and then pick from develop to release branch. But Maybe I am missing some context here, please correct me if I'm wrong.

@chaitanyaprem
Copy link
Contributor Author

Also ideally we should land things in develop and then pick from develop to release branch. But Maybe I am missing some context here, please correct me if I'm wrong.

these already reached develop but as part of different PRs #5653 (refactor) and others. Hence had to cherry-pick fixes that are required for the release.

@siddarthkay
Copy link
Contributor

Thanks for explaining @chaitanyaprem

@siddarthkay siddarthkay merged commit b8ceede into release/0.182.x Aug 9, 2024
7 checks passed
@siddarthkay siddarthkay deleted the fix/filter-connection-change branch August 9, 2024 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants