Cosmosim table updates

[agroener]

This PR will deal with all changes to how tables are presented to the user, how and when they get stored into a dictionary, and all related updates to the code.

[keflavich]

I just added to alma and eso the option store_password=True, which I think I will change to store_password=False for security reasons. This will select whether or not to store the password in keyring. Want to do that here too?

[agroener]

Definitely! I'll take a look at what you did.

[agroener]

Have you merged into master yet? I'm not seeing where you've added the option.

[keflavich]

No, not yet - brand new as of now +/- ish

[agroener]

Okay - just added it to mine in the same way.

[agroener]

Okay - Having an issue with _request for methods with method 'DELETE'. Looked into the code and only found GET and POST implemented. Could this be remedied?

[keflavich]

No, for those I think you're best off leaving the original request.delete in place - that's why I left those unchanged in my PR.

[agroener]

Oh okay - It was failing when those were left in, so I figured you'd forgotten those. Let me try to put it back and fix it up..

[keflavich]

It was failing on those? That's odd, I wonder if I messed something up at a higher level.

[agroener]

Yea - I need to do a bit of debugging. I've gotten delete_all_jobs() and delete_job() to not error (went back to self.session.delete() for the deletion http requests), but for some reason they're not actually getting deleted from the server.

[keflavich]

Oh. I can't really help with server-side things, but I'll try and dig through requesty things tomorrow

[agroener]

Well, I don't believe it's really a server side issue (I can go to the website and delete jobs just fine). Somehow the addition of _BaseQuery__session with requests.session() has complicated things.. Where is that coming from, by the way? I want to stare at that for a bit. I've already looked at QueryWithLogin (in query.py) and also in commons.py, but no luck.

[keflavich]

_BaseQuery__session comes from astroquery/query.py:BaseQuery, where it is defined as __session, but because it's an Abstract Base Class (ABC), it gets _BaseQuery prepended. At least, that's how I understand it.

[keflavich]

I actually think it is not possible - and probably should not be possible - to login once a session has been logout'd. Instead, the user should be required to make a new CosmoSim instance. So, perhaps remove the re-creation of the session in the login and instead check whether a session exists; if it doesn't, raise an Exception.

[agroener]

Okay - no need to respond to this immediately, I'm running some more tests. I'm getting some really odd behavior, but I think it's a simple problem. I'm able to run queries and delete them from the server (I'm manually checking the website), but for some reason, somewhere in my code it's not updating any of the jobs at all. I need to look into where exactly there is a disconnect...

[agroener]

I think perhaps the caching of the _requests method is messing things up. I originally had 4 or so jobs on the server, and after deleting and running a few more, the new method (with _requests as oppoed to session.post) it doesn't know about jobs which have been deleted or created since the first time I've used it. I know it's not issues on the server because I can access new jobs and delete them with the old method. Any ideas what's happening?

EDIT: Setting cache=False in most of the _request() calls returns the functionality back to normal (however, may defeat the whole purpose of making this change in the first place). I'll wait for a response or an update to the code.

[keflavich]

@agroener - It's worth giving some thought to what can and can't be cached. It's worth caching any requests that directly return tables or data files. Any requests that activates something on the server side - e.g., a staging request - should not be cached. Anything that is cached prevents transmission of the request whenever it's repeated.

[agroener]

Right okay. I was thinking about this last night. I have a couple of different jobs that I run:

1. Checking things: job creation, job phase, etc:
   I think these should not be cached, since the user will be creating and deleting jobs, and the list of jobs and their phases (and tablenames), has to change on the fly.
2. Creating jobs: sending an sql query to the server and creating a job:
   My first instinct is for this to be cached. Does caching prevent the user from ever having to run the same query again? How does that work?
3. Deleting jobs: deleting or aborting:
   I have no idea about this one. Perhaps no caching on this?

[keflavich]

Anything that is intended only to trigger a response on the server, i.e. (1) and (3), should not be cached. Anything where the server returns data should be cachable, so (2), as you suggested.

[agroener]

Okay, I've made the appropriate changes, and things seem to be functioning properly, though I'm a bit confused about the extra features _requests() has.

I thought caching meant it would prevent me from running the same query (or download the same data) twice? Also, what other features does _requests() have? I think I saw progressbar functionality in some of the code.

[keflavich]

If you download a large file, you can use _download_file or _request('GET', file_url, save=True) and it will stream the data with a download progressbar.

Yes, caching means that, if you send the exact same query to the same URL, it will first hash those parameters (basically, take all the data you're sending and turn it into a unique string of characters...), then search your cache directory for a pickle file matching that hash. If it finds the local version, it will skip sending anything to the remote server and instead just open up the saved file on disk.

[agroener]

Okay. I'll take a look at the download functionality. I have a few different formats I will most likely need (and in fact, I can get them from the server simply by using different url's). Interesting about caching. It's hard for me to know if it's working properly. On my end all I see is a second job being created, with phase executing on the server, and then a moment later it finishes. Are there are any tests I could come up with to see if things are working the way they should?

[keflavich]

Well, just make sure the outputs are the same both times.

What does 'phase executing on the server' mean? If a message is being sent to the server, you probably need to cache a second step.

[agroener]

From what I understand EXECUTING means that the sql query is actually executing on the server, which to me, indicates that caching is not working since it's running the exact same query. But honestly I really don't know for sure - I am certainly no expert.

I just can't really figure out how to see caching in action, I guess I just have to trust that it's happening. :)

[keflavich]

we can add a line in _request that says whether it's cached or not if log.setLevel(logging.DEBUG) is set

[agroener]

Yea, that would be good I think. You mentioned you were using astropy's logging, right?

[agroener]

One more thing: I'm testing out my explore_db function, and I need a json attribute to be enabled in _request():

response = self._request('GET', CosmoSim.SCHEMA_URL,
                             auth=(self.username,self.password),
                             headers={'Accept': 'application/json'},
                             cache=True)
data = response.json()

I should be able to add this myself in query.py, correct?

[keflavich]

Actually, you shouldn't need to any more - it should just be there (once you're rebased/merged with master)

[agroener]

Okay merged. Note: I think you deleted the 'auth' portion (wherever that was located) because now I cannot log in. Next thing I'm going to work on after this is creating tests... I now see why they're important.

[agroener]

Would the package 'prettytable' be okay to use, or do you discourage the use of it?

https://code.google.com/p/prettytable/wiki/Tutorial

[jwoillez]

Did you try astropy's Table.pprint()?

https://astropy.readthedocs.org/en/v0.2.1/_generated/astropy.table.table.Table.html#astropy.table.table.Table.pprint

[agroener]

@jwoillez Haven't looked into that yet; thanks for the pointer.

[agroener]

@keflavich Was _BaseQuery__session removed? I needed to rebase, and when I did all of a sudden it was complaining about that. I've commented out those lines and everything seems to work once again.

[agroener]

Okay. This should be good to go. I've added quite a bit here.

[keflavich]

@agroener - the Session change was from #435. Reviewing now.

[keflavich]

I think this needs to be changed now? Perhaps del self.session alone is enough.

[keflavich]

Only minor changes. I'm willing to merge this and accept future changes as bugfix pull requests. That OK with you @agroener ?

[agroener]

Okay - fixed that remaining session type in logout. If this passes, merge away.

When you say future changes as bugfixes, do you mean all cosmosim features, or just things pertaining to this _BaseQuery__session -> session switch ?

[keflavich]

Anything and everything. Send fixes as you see that they're needed.