-
Notifications
You must be signed in to change notification settings - Fork 234
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
add first round of pgtools #642
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOVE IT.
tools/pg_tools/macros.xml
Outdated
</token> | ||
|
||
<token name="@ARCHIVE_DATABASE@"><![CDATA[ | ||
tar -cvJf postgresql_out.tar.xz ./postgresql |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the html extra files path thing to avoid the overhead of tarring / un-tarring.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Retracted: out of band discussion concluded that disk space was more important than the computational overhead.
tools/pg_tools/macros.xml
Outdated
|
||
<token name="@PG_START@"><![CDATA[ | ||
'$__tool_directory__/pglite' start -d ./postgresql && | ||
sleep 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
slightly worried about this. Wonder if we could switch a while loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could put the sleep in a while look and ask if the server is available? Not sure what the best way is here. Maybe we could make this start command blocking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love a sleep in a while. Maybe something like the following would be good so we'd be safe from sleeping too much or too little, and we can be sure we won't sleep forever if something goes wrong.
exec_count=0
'$__tool_directory__/pglite' status | grep -F -q 'server is running'
while [ $? -ne 0 ]; do
sleep 2
exec_count=$((exec_count+1))
if [[ "$exec_count" -gt 10 ]]; then exit 1; fi
'$__tool_directory__/pglite' status | grep -F -q 'server is running'
done
(Completely untested.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used unless ... status
still does not work reliably, but checking for the socket file in addition does the trick.
tools/pg_tools/pg-query.xml
Outdated
@@ -0,0 +1,48 @@ | |||
<tool id="pg_query" name="Postgresql" version="@PG_VERSION@"> | |||
<description>import sql dump</description> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
? I don't see it :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change description from "import sql dump" to something like "query postgres database"
tools/pg_tools/pg-query.xml
Outdated
<param format="postgresql" name="infile" type="data" label="Input database" /> | ||
<param name="query" type="text" area="True" size="5x50" label="The query to be issued to the database"> | ||
<sanitizer> | ||
<valid initial="string.letters,string.digits"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this won't be enough, we'll need '
at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed, other can be at any time.
tools/pg_tools/pg-query.xml
Outdated
@PG_START@ && | ||
ls -l ./postgresql | ||
&& | ||
@PSQL@ -c '$query' -L logfile.log > '$outfile' && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we wrap this in a copy ($query) to stdout with csv delimiter ' ';
(i.e. raw tab char)? Then you'd get automatic tabular files. (or at least make it an option and make tab output default)
I added a new test and addressed your other concerns, besides the sleep thing. |
tools/pg_tools/pglite
Outdated
|
||
######################### Delegates to subcommands or runs main, as appropriate | ||
|
||
main "$@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to document the license / source+license for this script somewhere, i.e. https://github.com/solidsnack/pglite/blob/master/LICENSE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should ship this tool at all, we should rather depend on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works for me! (But we still need to either remove the script or add the license header)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed this file and we now depend on a conda package
So tests are passing. Its a start I guess ... |
tools/pg_tools/macros.xml
Outdated
</token> | ||
|
||
<token name="@STATUS_RUNNING@"><![CDATA[ | ||
timeout 30 bash -c 'until pglite status -d ./postgresql | grep -F -q "server is running"; do sleep 1; done' && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just fyi, the reason for my horribly long while loop was that timeout is unavailable on OSX. Not sure if we care about those guys (I don't really that much.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't care that much either and doubt that anyone one is running a server under OSX.
I think this is ready to go for a first version.
Like the additional tool! |
Ready to start things on a postgis version with @ValentinChCloud and @Alanamosse for Galaxy-E! |
@yvanlebras awesome. Let me know if these tools work for you and what I can do to improve it. |
Have you a postgresql datatype definition already ? |
Thanks @erasche ! I missed it ! Nice end of week. |
Ok, let's get this in. I'm currently trying to ship with a GIS and rdkit enabled PG version, but this can also be an update. Thanks everyone! ping @abretaud |
ping @annefou, this might be interesting for you as well. It can use GIS-enabled postgresql databases in user-space. You can treat a PG database like any other Galaxy database. |
This is something we have to work on. Maybe it's time to go further and try to test creating a qgis 3 interactive took! |
Yes. We can discuss it during the co-fest?! |
Sure :) |
Yes! Very good idea Anne. I will try to have some GIS colleagues, maybe you have this ? |
Yes we use arcGIS a lot but more and more QGIS too. We have dockerized versions of QGIS desktop that could be added to Galaxy. I am not sure what is best so we can discuss it during the co-fest! |
I was notably thinking about something like thishttps://github.com/jancelin/geo-poppy |
This will use a in-user-space postgresql version and can load schemas into it and query the db. Many more tools will be added in the future. Open for comments.