chandra-mta / space_weather

All space weather monitoring. ACE, GOES, KP.
1 stars 0 forks source link

Linux #5

Closed bspitzbart closed 9 years ago

bspitzbart commented 9 years ago

This pull makes ACE monitoring (calculate.csh) compatible on Linux. Also we update to the new SWPC links. No changes are needed in the supporting programs (process_ace.nawk and aceviolation_protons.csh) The process was tested and is running as a cronjob on c3po-v, producing output at: https://cxc.cfa.harvard.edu/mta_days/MIRROR/ace.html (The format of this page is identical to the current one.)

I tested the alert by setting the limit to 0 and successfully sent myself a message (via cron). We would like to implement on the primary ACE page next Wednesday or Thursday.

taldcroft commented 9 years ago

@bspitzbart - apart from my final comment about wingkp*.gif files this looks fine from the code perspective. You should run your tests on the "final" version and then submit a change request form to Flight Director.

taldcroft commented 9 years ago

@bspitzbart - also, when you add new commits to address comments it is useful to ping the reviewers with a github comment. Otherwise there is no notification that anything has happened. You can also do something trickier, which is to include the @username in your commit, for instance:

git commit -am "Clean up obsolete (deleted) lines per comment from @taldcroft"

That will send me an email with the commit.

jeanconn commented 9 years ago

What happened with the duplicate 'convert' on Epam_7di.gif issue?

bspitzbart commented 9 years ago

The second convert is removed. Is there still an issue?

On Fri, Feb 20, 2015 at 10:23 AM, Jean Connelly notifications@github.com wrote:

What happened with the duplicate 'convert' on Epam_7di.gif issue?

— Reply to this email directly or view it on GitHub https://github.com/mta/space_weather/pull/5#issuecomment-75255113.

jeanconn commented 9 years ago

I still see what are now lines 65 and 68 both calling convert (one in a pipe at the end of the line, one at the beginning) to finish making Epam_7di.gif.

bspitzbart commented 9 years ago

Ah, thanks. It is now removed.

On Fri, Feb 20, 2015 at 10:39 AM, Jean Connelly notifications@github.com wrote:

I still see what are now lines 65 and 68 both calling convert (one in a pipe at the end of the line, one at the beginning) to finish making Epam_7di.gif.

— Reply to this email directly or view it on GitHub https://github.com/mta/space_weather/pull/5#issuecomment-75258043.

jeanconn commented 9 years ago

Great. You could probably also reduce that to one "lynx" operation and then convert and copy as needed, but that is obviously not required.

jeanconn commented 9 years ago

Out of curiosity, where does mta_ace_plot_P3.gif come from? I don't see it created around where it is used.

bspitzbart commented 9 years ago

It comes from another stand alone (IDL) script that we are also working on fixing (hence, it is not currently being used).

On Fri, Feb 20, 2015 at 10:50 AM, Jean Connelly notifications@github.com wrote:

Out of curiousity, where does mta_ace_plot_P3.gif come from? I don't see it created around where it is used.

— Reply to this email directly or view it on GitHub https://github.com/mta/space_weather/pull/5#issuecomment-75259990.

taldcroft commented 9 years ago

@bspitzbart - now that this has been approved you can merge here (click the Merge button) and merge to master locally, then install from master:

cd ~/git/space_weather  # or wherever you have it
git fetch origin
git checkout master
git merge origin/master
# Install however you do it
git branch -d Linux
bspitzbart commented 9 years ago

I have installed the patch and verified http://cxc.cfa.harvard.edu/mta/ace.html is updating correctly.

On Fri, Feb 20, 2015 at 6:57 PM, Tom Aldcroft notifications@github.com wrote:

@bspitzbart https://github.com/bspitzbart - now that this has been approved you can merge here (click the Merge button) and merge to master locally, then install from master:

cd ~/git/space_weather # or wherever you have it git fetch origin git checkout master git merge origin/master

Install however you do it

git branch -d Linux

— Reply to this email directly or view it on GitHub https://github.com/mta/space_weather/pull/5#issuecomment-75341523.