Skip to content

Allow index_netcdf to reference relative paths#61

Merged
jameshiebert merged 1 commit intomasterfrom
feature/abspath
Mar 8, 2018
Merged

Allow index_netcdf to reference relative paths#61
jameshiebert merged 1 commit intomasterfrom
feature/abspath

Conversation

@jameshiebert
Copy link
Copy Markdown
Contributor

A common use case for indexing a set of files is to cd into the local
directory containing the files and then to run "index_netcdf *". This
will fail, unfortunately, because the database tracks absolute paths
and index_netcdf naively accepts the paths from the command line.

This PR tells index_netcdf to use the normalized absolutized version of
each path, so that the command line args will match up with the paths
in the database.

@rod-glover
Copy link
Copy Markdown
Collaborator

rod-glover commented Feb 23, 2018

Your changes LGTM.

The failing tests are clearly because the test framework isn't freeing up database resources after tests:

E OperationalError: (psycopg2.OperationalError) FATAL: sorry, too many clients already
E (Background on this error at: http://sqlalche.me/e/e3q8)

The pattern of failures w.r.t. Python versions is I think arbitrary.
I think these test problems are a matter for a separate issue/PR.

@jameshiebert
Copy link
Copy Markdown
Contributor Author

because the test framework isn't freeing up database resources after tests

Have ideas on how to address that?

@rod-glover
Copy link
Copy Markdown
Collaborator

Yes. I've been improving my test code successively in various database projects. I'm not sure where this one is in the sequence, and I'll have to have a look around to find which repo has the latest "version", but I think I know what to do here and have existing code that does it somewhere.

@jameshiebert
Copy link
Copy Markdown
Contributor Author

jameshiebert commented Feb 23, 2018

Cool. I have an illogically visceral reaction to merging a PR with failing tests (even if it's completely unrelated). So I'll hold off on the merge, and rebase after we get your test fixes.

@rod-glover
Copy link
Copy Markdown
Collaborator

Turns out I encountered and fixed the problem in the other PR open on this repo. I'll address your comments and get that merged, then we can rebase our ongoing stuff onto that.

A common use case for indexing a set of files is to cd into the local
directory containing the files and then to run "index_netcdf *". This
will fail, unfortunately, because the database tracks absolute paths
and index_netcdf naively accepts the paths from the command line.

The PR tells index_netcdf to use the normalized absolutized version of
each path, so that the command line args will match up with the paths
in the database.
@jameshiebert
Copy link
Copy Markdown
Contributor Author

After a rebase, all looks good. Merging.

@jameshiebert jameshiebert merged commit f04ff87 into master Mar 8, 2018
@jameshiebert jameshiebert deleted the feature/abspath branch March 8, 2018 21:23
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.

2 participants