Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adds access to archive database to sdssdb #11

Merged
merged 30 commits into from
Sep 24, 2019
Merged

adds access to archive database to sdssdb #11

merged 30 commits into from
Sep 24, 2019

Conversation

joelbrownstein
Copy link
Contributor

Hi Brian,

I've run pg_restore on the archive database on manga.sdss.org, and installed the archive branch of sdssb on Utah system-wide for this PR.

Can you confirm the following works (or I may need to fix the postgresql schema/table permissions on your userid)?

[u0707758@manga:~]$ module load sdssdb/archive
[u0707758@manga:~]$ module switch python python/3.6.3
[u0707758@manga:~]$ ipython
Python 3.6.3 (default, Oct 20 2017, 13:08:04)
Type 'copyright', 'credits' or 'license' for more information
IPython 6.2.1 -- An enhanced Interactive Python. Type '?' for help.

In [1]: from sdssdb.sqlalchemy.archive.sas import *

In [2]: database
Out[2]: <ArchiveDatabaseConnection (dbname='archive_20190507', profile='manga', connected=True)>

In [3]: session = database.Session()

In [4]: for tree in session.query(Tree).all(): print(tree.version)
bosswork
apogeework
ebosswork
mangawork
sdsswork
dr7
dr8
dr9
dr10
dr11
dr12
dr13
dr14
dr15

In [5]: for tree in session.query(Tree).all():
     try: env = session.query(Env).filter(Env.tree_id==tree.id).filter(Env.label=='MANGA_SPECTRO_REDUX').one()
     except: env=None
     if env:
         directory = session.query(Directory).filter(Directory.env_id==env.id).order_by(Directory.nested).first()
         print("TREE> {version} has env {label} with topdir {path} with directory_count={directory_count}, file_count={file_count}".format(version = tree.version, label =
  env.label, path = directory.path, directory_count = directory.nested_directory_count, file_count = directory.nested_file_count))

TREE> dr13 has env MANGA_SPECTRO_REDUX with topdir /uufs/chpc.utah.edu/common/home/sdss00/dr13/manga/spectro/redux with directory_count=879, file_count=85728
TREE> dr14 has env MANGA_SPECTRO_REDUX with topdir /uufs/chpc.utah.edu/common/home/sdss08/dr14/manga/spectro/redux with directory_count=1956, file_count=189510
TREE> dr15 has env MANGA_SPECTRO_REDUX with topdir /uufs/chpc.utah.edu/common/home/sdss08/dr15/manga/spectro/redux with directory_count=7132, file_count=553326

In [6]:

Cheers,
Joel

Joel Brownstein and others added 3 commits August 10, 2019 15:29
Copy link
Collaborator

@havok2063 havok2063 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joelbrownstein I had a quick check with the example you gave. Two things.

  1. When I first imported the database it was not connected to profile manga, as in your example
from sdssdb.sqlalchemy.archive.sas import *
database
<ArchiveDatabaseConnection (dbname='archive_20190507', profile='manga', connected=False)>

I had to set the profile to local first

database.set_profile('local')
database
<ArchiveDatabaseConnection (dbname='archive_20190507', profile='local', connected=True)>

Was that the expected behavior?

  1. I am having permissions issues. I got

session.query(Tree).all() and got
ProgrammingError: (psycopg2.ProgrammingError) permission denied for schema sas

@havok2063 havok2063 changed the title Archive adds access to archive database to sdssdb Aug 23, 2019
@joelbrownstein
Copy link
Contributor Author

joelbrownstein commented Aug 23, 2019

I ran

# GRANT ALL PRIVILEGES ON SCHEMA sas TO GROUP sdssuser;
GRANT
# GRANT ALL PRIVILEGES ON ALL TABLES IN SCHEMA sas to sdssuser;
GRANT

can you re-try, please?

@joelbrownstein
Copy link
Contributor Author

I thought local would be the default behavior

@havok2063
Copy link
Collaborator

@joelbrownstein that worked for the permissions, at least with the given example. I think by default, with autoconnect=True, sdssdb will try to find an appropriate profile to use based on the hostname. So in this case, it finds a manga profile for the manga host. I'm not sure why yours connected and mine does not.

@joelbrownstein
Copy link
Contributor Author

ok on permissions!

and I think you are right about manga, so we need to specify local on that machine.

@havok2063
Copy link
Collaborator

@joelbrownstein I think it's not connecting because I don't have the archive db set up in my .pgpass file for manga.wasatch.peaks.

@joelbrownstein
Copy link
Contributor Author

manga.wasatch.peaks is the infiniband address that the cluster uses, so I don't think you want that

Copy link
Contributor Author

@joelbrownstein joelbrownstein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think some of these relationships need to have the backref pluralized, in those cases where the relationship is many to one.

@nbmurphy
Copy link
Contributor

nbmurphy commented Sep 5, 2019

The relationship changes that I pushed to the archive branch made my archive_20190507 database connection not work correctly; the Python code used to query the db does not work with the relationships and works correctly without.

@havok2063
Copy link
Collaborator

havok2063 commented Sep 18, 2019

@joelbrownstein @nbmurphy I cannot seem to pull the latest changes at Utah to start looking at this. I'm the manga user looking at /uufs/chpc.utah.edu/common/home/sdss10/software/sdss/github/sdssdb/archive. Is this the right place?

Also, @nbmurphy can you provide an error message of the type of problem you are getting?

@joelbrownstein
Copy link
Contributor Author

joelbrownstein commented Sep 18, 2019

I ran git pull, can you try now? I can add a crontab to keep it updated, or we can run sdss_install as the manga user?

@havok2063
Copy link
Collaborator

It looks updated. Thanks.

@havok2063
Copy link
Collaborator

havok2063 commented Sep 18, 2019

@joelbrownstein can I (manga) have access to edit the directory? I need to debug some stuff in real time for the archive models.

Copy link
Collaborator

@havok2063 havok2063 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joelbrownstein @nbmurphy I've pushed some changes to fix the relationships. It gets a bit funky because some columns are keys that reference itself. Other tables have multiple columns that point to the same foreign key, in which case you need to be more explicit with sqlalchemy about how they join together. I've also added some print_fields so when loading the models in python you get a bit more info on what the thing is. Please have a look and let me know what you think. Otherwise this seems to work now at Utah. @joelbrownstein You can pull the changes into your version of sdssdb.

@nbmurphy
Copy link
Contributor

nbmurphy commented Sep 20, 2019

The new changes work with my existing code that implements sdssdb-archive. I also added the requested documentation.

Copy link
Member

@albireox albireox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I requested some changes. Also note that I made a number of changes to master so you'll need to merge those and maybe fix some conflicts.

Also, never too late to link this to an issue ...

python/sdssdb/sqlalchemy/archive/__init__.py Show resolved Hide resolved
python/sdssdb/sqlalchemy/archive/__init__.py Show resolved Hide resolved
docs/sphinx/intro.rst Outdated Show resolved Hide resolved
@havok2063
Copy link
Collaborator

@albireox your recent changes in master have broken the ability to run database.set_profile('local') to connect to a database as localhost.

To reproduce,

  • login to the manga machine
  • try to connect to the archive database
from sdssdb.sqlalchemy.archive import database, sas
database.set_profile('local')

@havok2063
Copy link
Collaborator

@albireox Your recent changes in master regarding setup.cfg and the requirements files have also broken the ability for Travis to run. See the above Travis link for details. If this is a permanent global change/simplification then the python_template and relevant docs should be updated accordingly.

@albireox
Copy link
Member

Yes, the use of setup.cfg is something I'd like to add to the template. I've been experimenting a bit with poetry and other such system and long story short I think using a setup.cfg is the way to go. I like how it consolidates things and makes the management of requirements much more natural.

Yes, there is a ticket for the Travis issue, #18. Since we really don't have tests I hadn't fixed it yet. It should be easy to and maybe you already have.

How are the changes breaking the use of set_profile? Your example works for me but it does not connect to the DB, as expected because that database is not local to the manga machine, I think.

@havok2063
Copy link
Collaborator

havok2063 commented Sep 23, 2019

@albireox
Yeah I've used poetry for a few projects. I quite like it but pip+setup.cfg is still a more ubiquitous solution. Your changes definitely improve our existing setup. I like it. At some point we might want to consider moving to a pyproject.toml file. I think that's the recommended standard in the latest PEP codes.

I've fixed the Travis issues here to at least get it to run. I'm working on some ideas on how to improve database testing generally. Once I sort it out I'll submit a new PR.

There is a local archive database currently on the manga machine, archive_20190507 which is currently set as the dbname in the ArchiveDatabaseConnection. I can psql into it from the terminal. Before merging, setting database.set_profile('local') successfully connected to the database. Now it fails to connect with not much verbosity.

@albireox
Copy link
Member

Yes, at some point we should use a pyproject.toml. The main problem is that it's still not the default. Same with poetry; pip removed its support for PEP-517 (see here) so I think poetry is not usable for us right now. Maybe in the future.

I'll have a look at the issue with set_profile. I didn't really change much apart from explicitly setting host=localhost and port=5432. I also modified .connect a bit so that you can override values from the profile, but that should not break the set_profile.

@albireox
Copy link
Member

So, the problem happens because I added specifically that host=localhost for the local profile. That seems reasonable but at utah that fails

psql -h localhost archive_20190507
psql: FATAL:  no pg_hba.conf entry for host "::1", user "u0931042", database "archive_20190507", SSL off

but for some reason just psql archive_20190507 works. This seems like a problem with how the VM is configured and I feel it will change from setup to setup.

I don't remember exactly why I added the host specifically. I think it was because when I was trying to use the local profile for a tunnel connection it would fail unless I passed host='localhost'. Since we are kind of dealing with exceptions here I would be ok removing the lines specifying host and port from the local profile.

@havok2063
Copy link
Collaborator

That makes sense. I agree this will be machine setup specific. Maybe we can leave the local profile without the parameters for now and come up with another solution for tunneling. Thanks for looking into it.

@joelbrownstein
Copy link
Contributor Author

I've allowed unix sockets via a few selected unids for convenience, which is why it works for you without -h and without -U. However once you add -h, then the connection string should also specify the shared user via -U sdss

@havok2063 havok2063 merged commit 3b5f6ad into master Sep 24, 2019
@havok2063 havok2063 mentioned this pull request Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants