write weathers emails to a file

 so that it doesn't spam regular users during testing.
 

Trac:
Username: feverDream

added component::metrics/tor weather owner::feverDream priority::medium reporter::feverDream resolution::wontfix status::closed type::enhancement weather-rewrite labels

Trac:
Username: feverDream
Status: new to assigned
Type: task to enhancement

Trac:
Username: feverDream
Keywords: N/A deleted, weather-rewrite added

Trac:
Cc: N/A to karsten

emails are sent via send_mass_emails function in updaters.py. So commented it out and the email tuples are written to a file in the applications log directory. Added a link to commit below.

https://github.com/achintangal/weather/commit/271f59755bbd0f1f4d546959a5f203f9210fbc0c

Trac:
Username: feverDream

Commented on GitHub. Thanks!

Actually, I shouldn't have commented on GitHub, but here. Moving the discussion here again.

So, here's some feedback on https://github.com/achintangal/weather/tree/68035eeea9d26c7f64efd9dbe54845ac4f0e1430:

It looks like Django already provides a convenient way to write email to a file rather than sending it out: https://docs.djangoproject.com/en/1.4/topics/email/#file-backend. Huh! Mind trying that out?

If it works, we shouldn't hack something ourselves. How about we comment out the current email settings that send out actual mail in settings.py and instead put in the file email backend as default? So, whenever we set up weather for local testing or for testing in a local VM using Vagrant, emails would be written to a file. And when we deploy on the server, we'd put in real SMTP settings.

By the way, I somehow like the idea of using different settings files in a settings/ directory for the different environments. I'm just not sure if it's overkill for weather, because the environments aren't that different. But in any case, this discussion deserves a ticket of its own, and the code changes deserve their own branch. Mind opening a ticket and move the discussion there?

Nice, I think I should pay more attention to the Django documentation :)

About the settings, I too found the separate module approach a bit clean but I figured its better to ask the other members of the team and see if its actually required.

Trac:
Username: feverDream

karsten,https://github.com/achintangal/tor-weather-rewrite/commit/395a16ef6f2e0c16fef59de2b9dfb090634f8b93

this issue is also fixed with the same commit[1].

[1]https://github.com/achintangal/tor-weather-rewrite/commit/395a16ef6f2e0c16fef59de2b9dfb090634f8b93

Trac:
Username: feverDream

There are a few issues with that commit:

• We cannot comment out settings in weather/config/config.py until we actually moved to Onionoo. What if somebody takes this branch and tries to deploy Weather? It won't work at all.
• Rather than commenting out code or settings, we should simply remove them. That's for the future, though, when we can actually get rid of them.
• I'm unclear why you changed the path to the database. That's going to break the existing deployment whenever somebody does git pull. We shouldn't do that. We should rather stick to the existing directory layout with /../var/ even though it's not pretty.
• Maybe we should also write emails somewhere to /../var/? It seems that Weather currently doesn't write anything to the cloned directory root, but only to /../var/. If that is the case, let's not clutter up /..

So, I guess a commit that only adds the EMAIL_BACKEND and EMAIL_FILE_PATH lines to settings.py but with the right directory to write emails to would be fine to merge.

Replying to karsten:

There are a few issues with that commit:

• We cannot comment out settings in weather/config/config.py until we actually moved to Onionoo. What if somebody takes this branch and tries to deploy Weather? It won't work at all.
• Rather than commenting out code or settings, we should simply remove them. That's for the future, though, when we can actually get rid of them.

Makes sense, but Django expects these files to be a there. I can either go with a dummy authenticator file or a "except: pass" loop around it. I am in favor of the latter.

• I'm unclear why you changed the path to the database. That's going to break the existing deployment whenever somebody does git pull. We shouldn't do that. We should rather stick to the existing directory layout with /../var/ even though it's not pretty.

I remember changing it as the path was incorrect. To make it work I made these changes;changed the path to PROJECT_PATH:/../../var/WeatherDB and change the permissions of the db-file. For development purposes, I figured the db-file in '/.' was simple.

To keep things intact, making these changes local makes sense to me. What do you think? I can add a few steps in the README describing the settings that need to be changed to run weather locally.

• Maybe we should also write emails somewhere to /../var/? It seems that Weather currently doesn't write anything to the cloned directory root, but only to /../var/. If that is the case, let's not clutter up /.. Sure thing, I will look into it.

So, I guess a commit that only adds the EMAIL_BACKEND and EMAIL_FILE_PATH lines to settings.py but with the right directory to write emails to would be fine to merge.

Fine, I will trim the commit and move the "settings" stuff to their own ticket( should keeps things separate :))

Trac:
Username: feverDream

Tor Weather has been discontinued as of May 24, 2016: https://lists.torproject.org/pipermail/tor-relays/2016-June/009424.html. Batch-closing all remaining tickets as announced in #19382 (moved). A list of these tickets and any other Weather tickets modified after June 26, 2016 will be available here: https://trac.torproject.org/projects/tor/query?changetime=Jun+27%2C+2016..&component=^Metrics%2FTor+Weather

Trac:
Status: assigned to closed
Resolution: N/A to wontfix

closed