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

db: make database path relative to config.core.homedir #1574

Merged
merged 1 commit into from
Aug 27, 2019

Conversation

HumorBaby
Copy link
Contributor

Makes the default base directory for the database core.homedir. Non-absolute paths are relative to that directory, as opposed to previously, where they were relative to the config file path.

Additionally, now SopelDB.__init__ will raise OSError if the directory in which the database is to be created does not exist.

Closes #1047.

@HumorBaby
Copy link
Contributor Author

Any @sopel-irc/rockstars takers to review this? I'm hoping its pretty straightforward 😁 we shall see...

@Exirel
Copy link
Contributor

Exirel commented May 8, 2019

@HumorBaby doesn't that is in conflict with #1446?

@Exirel Exirel added this to the 7.0.0 milestone May 8, 2019
@Exirel
Copy link
Contributor

Exirel commented May 31, 2019

The conflicts look... interesting. Good luck. :-/

@dgw
Copy link
Member

dgw commented May 31, 2019

On the plus side, there's only "one" conflict? 😕

@dgw dgw added Stale Mostly used for PRs that no longer work and need updating before re-review/merge. and removed Needs Review labels Jun 30, 2019
@HumorBaby HumorBaby force-pushed the refactor-db_filename-dir branch from 3b4c0ff to 701be15 Compare August 1, 2019 13:54
@HumorBaby
Copy link
Contributor Author

The conflicts look... interesting. Good luck. :-/

hehe, wasn't too bad actually! RustyB developed around the part I modified 😁 Anyway, the conflict is handled now.

@HumorBaby HumorBaby removed the Stale Mostly used for PRs that no longer work and need updating before re-review/merge. label Aug 1, 2019
Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

Almost 👍

sopel/db.py Outdated Show resolved Hide resolved
sopel/db.py Outdated Show resolved Hide resolved
sopel/db.py Outdated Show resolved Hide resolved
@HumorBaby HumorBaby requested a review from Exirel August 14, 2019 02:24
Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

Last very small nitpick, and it's approved!

sopel/db.py Outdated Show resolved Hide resolved
Raises `OSError` if the directory in which the database will be created
does not exist.
@HumorBaby HumorBaby force-pushed the refactor-db_filename-dir branch from 2885698 to 6b8afcf Compare August 14, 2019 12:44
@HumorBaby HumorBaby added the Bugfix Generally, PRs that reference (and fix) one or more issue(s) label Aug 14, 2019
@dgw dgw added Tweak and removed Bugfix Generally, PRs that reference (and fix) one or more issue(s) labels Aug 14, 2019
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

🎉

path = os.path.normpath(os.path.join(config_dir, path))
path = os.path.normpath(os.path.join(config.core.homedir, path))
if not os.path.isdir(os.path.dirname(path)):
raise OSError(
Copy link
Member

Choose a reason for hiding this comment

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

I could bikeshed this and say we should use FileNotFoundError, but 💤. It's an easy change to make later if we really want to.

@dgw dgw merged commit 413add7 into sopel-irc:master Aug 27, 2019
@HumorBaby HumorBaby deleted the refactor-db_filename-dir branch August 28, 2019 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

db_filename is relative to the directory where the config file resides
3 participants