e-mission / e-mission-docs

Repository for docs and issues. If you need help, please file an issue here. Public conversations are better for open source projects than private email.
https://e-mission.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
15 stars 34 forks source link

`push.json` not being loaded correctly for `Push Notification` #1061

Open jiji14 opened 8 months ago

jiji14 commented 8 months ago

While testing the push notification feature on the Admin dashboard, an error occurred due to the inability to load the push_config_file in notify_interface.py in the server code (see below).

try:
    push_config_file = open('conf/net/ext_service/push.json')
    push_config = json.load(push_config_file)
    push_config_file.close()
except:
    logging.warning("push service not configured, push notifications not supported")

Our initial assumption was that the push.json file might be missing in the staging environment. However, upon further examination of the error log, we confirmed that push.json does exist. Therefore, we need to investigate further to determine why it is not being loaded correctly.

jiji14 commented 8 months ago

@MukuFlash03 Can you please share the screenshots related to this issue?

MukuFlash03 commented 8 months ago

So I took a look at the logs in AWS for admin-dash staging and found the two errors in the latest logs. I believe these might be related to these two bugs you've mentioned during your initial observations:

[ Admin Dashboard Bugs ] Trip trend data doesn't show up on Home Push notification fails

Here are the respective screenshots of the logs:

  1. "NameError: name 'push_config' is not defined" Possibly related to: Push notification fails
Screen Shot 2024-03-20 at 11 35 40 AM
  1. "ValueError: time data" Possibly related to: Trip trend data doesn't show up on Home
Screen Shot 2024-03-20 at 11 42 23 AM
MukuFlash03 commented 8 months ago

Our initial assumption was that the push.json file might be missing in the staging environment. However, upon further examination of the error log, we confirmed that push.json does exist. Therefore, we need to investigate further to determine why it is not being loaded correctly.

The push.json file is supposed to be based off of the template / sample JSON file present in the e-mission-server.

So, we took a look at the internal server repository and saw that the analysis container does have the push.json file available with the actual values filled in place of the sample values.

I believe we were incorrect in assuming the purpose of this specific internal push.json file. We initially assumed that the file is present and despite being present it is not being loaded.

I believe, the correct explanation would be that this file is for the analysis container specifically hence it is not available to the admin-dashboard container. The admin-dashboard container does not have the push.json file in the internal repo which is where it builds the image from the base server image from the public version which in turn doesn't have the push.json file but only has a push.json.sample file.

So, need to figure out whether to add the file internally in the admin-dashboard repo?

@nataliejschultz For visibility, as it seems related to our redesign process in this issue.

jiji14 commented 8 months ago

@MukuFlash03 Thanks for the clarification! 👍 @achasmita @shankari Also for visibility

shankari commented 8 months ago

The push notifications were not expected to work.

So, need to figure out whether to add the file internally in the admin-dashboard repo?

Yes