Skip to content

Implement local/system configs#126

Merged
sjrct merged 2 commits into
masterfrom
local-system-configs
Feb 6, 2019
Merged

Implement local/system configs#126
sjrct merged 2 commits into
masterfrom
local-system-configs

Conversation

@sjrct

@sjrct sjrct commented Feb 5, 2019

Copy link
Copy Markdown
Member

This adds support for separate local and system configs. The principle use case is for globally installed packages that can be accessed by anyone.

Had to add a special variable to halibot configs. If this was not done, then there would no way to keep the system config pointing to the source package path without doing some substitution or something in install.sh, which is not ideal as it would not work out-of-the-box, only after installation.

Fixes #122 and fixes #121.

@sjrct
sjrct force-pushed the local-system-configs branch from 78b1a15 to a960744 Compare February 5, 2019 02:39
@sjrct
sjrct requested a review from richteer February 5, 2019 02:40
@coveralls

coveralls commented Feb 5, 2019

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-1.7%) to 93.021% when pulling a960744 on local-system-configs into e033e85 on master.

System config lives in the source directory. Added a special variable to
halibot configs. If this was not done, then there would no way to keep
the system config pointing to the source package path without doing some
subsitution or something in install.sh, which is not ideal as it would
not work out-of-the-box, only after installation.
Comment thread halibot/halibot.py
def _load_config(self):
# Special values expanded inside of configs
specials = {
'HALDIR': HALDIR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

of Lothlórien!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I was thinking HALPATH might be better, but you have convinced me otherwise.

Comment thread halibot/halibot.py Outdated
text = Template(f.read()).safe_substitute(**specials)
self.config.set_system(json.loads(text))
except FileNotFoundError as _:
self.log.info("No sytem config loaded.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

s/sytem/system

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oops

@richteer

richteer commented Feb 6, 2019

Copy link
Copy Markdown
Contributor

Largely looks good to me. I think we should go ahead and merge this one so some of the other recent changes can get merged on top of this one now.

I'll approve the changes but won't merge yet in case you want to fix that typo. Otherwise, just go ahead and merge, not a bit deal.

sytem -> system
@sjrct
sjrct merged commit 650af10 into master Feb 6, 2019
@sjrct
sjrct deleted the local-system-configs branch February 6, 2019 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants