Conversation
| # | ||
| cmd = 'curl -u tisobe:' + out | ||
| cmd = cmd + ' https://occweb.cfa.harvard.edu/occweb/FOT/engineering/thermal/' | ||
| cmd = f'curl -u {user}:{password} https://occweb.cfa.harvard.edu/occweb/FOT/engineering/thermal/' |
There was a problem hiding this comment.
This is probably not secure since it exposes the password in your processes while curl is running. Use kadi.occweb.get_occweb_page instead with binary=True.
There was a problem hiding this comment.
it seems like Tom had a low impact change. Do you want ot use that, or would you like to gow with this for now and change it later?
There was a problem hiding this comment.
I implemented the change suggested by Tom, see below.
|
The change suggested by Tom has been implemented. |
| # | ||
| if len(data) < 1: | ||
| cmd = 'rm ' + temp_opfile | ||
| cmd = f'rm {temp_opfile}' |
There was a problem hiding this comment.
Not a huge deal, but all these os.system commands can be replaced by Python built-in functions in https://docs.python.org/3.8/library/shutil.html and https://docs.python.org/3.8/library/os.html. There are move() and copy2() in shutil and remove in os.
The main problem here is that there is no real error handling in the current os.system(cmd) call. If that fails then a non-zero value is returned but that is being ignored in this code. At the least all the os.system(cmd) calls should be replaced by assert os.system(cmd) == 0 so an exception is raised for a problem.
3 participants
Updated occweb authorization when fetching glimmmondb.sqlite3. The old version was using credentials of an account that has been discontinued.
In addition, corrected a bug in various paths used for copying and moving the output files.