Fix silent push #987
Merged
Fix silent push #987
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As part of e-mission#980 (and e-mission@5a8ca40 in particular), we initialize the firebase module with the values we read from the config. However, we removed the initialization of the `server_auth_token` https://github.com/e-mission/e-mission-server/pull/980/files#diff-5a90853314b14f3bec03976fa2ba81ec657e6327b770e9b376cfeabc0544487eL24 This is no longer used to send the actual notification, but it is used to map the FCM tokens to APNS. Note this is working for some deployments (e.g. nrel_commute) but not for other (e.g. stage). I can confirm that I get reminder notifications on my personal android phone. I don't understand how this ever worked - before e-mission#980, we weren't using the HTTP v1 version so no notifications were sent. After the change, this should have prevented notifications from being sent out. Testing done. Before the change ``` Connecting to database URL ... Traceback (most recent call last): File "/usr/src/app/bin/push/silent_ios_push.py", line 24, in <module> response = pnu.send_silent_notification_to_ios_with_interval(args.interval, dev=args.dev) File "/usr/src/app/emission/net/ext_service/push/notify_usage.py", line 41, in send_silent_notification_to_ios_with_interval return __get_default_interface__().send_silent_notification(token_map, {}, dev) File "/usr/src/app/emission/net/ext_service/push/notify_interface_impl/firebase.py", line 194, in send_silent_notification fcm_token_map = self.convert_to_fcm_if_necessary(token_map, dev) File "/usr/src/app/emission/net/ext_service/push/notify_interface_impl/firebase.py", line 137, in convert_to_fcm_if_necessary importedResultList = self.retrieve_fcm_tokens(unmapped_token_list, dev) File "/usr/src/app/emission/net/ext_service/push/notify_interface_impl/firebase.py", line 98, in retrieve_fcm_tokens importHeaders = {"Authorization": "key=%s" % self.server_auth_token, AttributeError: 'FirebasePush' object has no attribute 'server_auth_token' ``` After the change ``` Config file not found, returning a copy of the environment variables instead... Retrieved config: {'DB_HOST': '...', 'DB_RESULT_LIMIT': None} Connecting to database URL ... Received invalid result for batch starting at = 0 after mapping iOS tokens, imported 0 -> processed 0 combo token map has 30 ios entries and 0 android entries Successfully sent to ... Successfully sent to ... Successfully sent to ... Found error Token not registered while sending to token ... skipping Successfully sent to ... Successfully sent to ... Found error Token not registered while sending to token ... skipping Successfully sent to ... Successfully sent to -3sW... Found error Token not registered while sending to token ... skipping Successfully sent to ... ```
We check to see if a user has valid trips in the past 7 days before we send them a reminder (e-mission@7034479). However, all the related messages are displayed using `logging.debug`. Because of the additional imports, logging is configured to WARNING, so they are never displayed. And in the case where there are no users with recent trips, this results in insufficient logging to determine what happened. - Changing the logging configuration location - Changing the log level to INFO - Bumping up some useful logs to INFO Testing done: Before the change ``` Found configuration, overriding... Activating the environment... Run trip labeling reminder... Config file not found, returning a copy of the environment variables instead... Retrieved config: {'DB_HOST': '...', 'DB_RESULT_LIMIT': None} Connecting to database URL ... WARNING:root:Push configured for app .... using platform firebase with token ... of length 152 ``` After the change ``` Found configuration, overriding... Activating the environment... Run trip labeling reminder... Config file not found, returning a copy of the environment variables instead... Retrieved config: {'DB_HOST': '...', 'DB_RESULT_LIMIT': None} Connecting to database URL ... WARNING:root:Push configured for app ... using platform firebase with token ... of length 152 INFO:root:Successfully downloaded config with version 1 for Staging environment for testing programs only and data collection URL https://openpath-stage.nrel.gov/api/ INFO:root:No users to notify in lang en ```
|
We should also add some tests for |
|
Events are now being generated on staging |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In #980, we migrated the push code over to HTTP v1 of the FCM API.
This appeared to work then, and is still working now for some deployments
Working (nrel-commute)
or
Not working (ccebike)