Use hook template from layers instead of embedding #75

Merged
merged 4 commits into from Dec 14, 2015

Conversation

Projects
None yet
5 participants
Member

johnsca commented Dec 8, 2015

Implement #74. This will allow iteration on the hooks w/o release of charm-tools, as well as non-Python base layers.

charmtools/build/__init__.py
+ self.target, target_config, template))
+
+ def plan_storage(self, layers, output_files, plan):
+ # Interface includes don't directly map to output files
@bcsaller

bcsaller Dec 8, 2015

Contributor

copy-paste error on comment

@@ -119,6 +115,7 @@ class Builder(object):
Handle the processing of overrides, implements the policy of BuildConfig
"""
PHASES = ['lint', 'read', 'call', 'sign', 'build']
+ HOOK_TEMPLATE_FILE = path('hooks/hook.template')
@marcoceppi

marcoceppi Dec 8, 2015

Owner

It seems odd that we define this several times. Here, L12 & L202 of tactics.py, etc

@johnsca

johnsca Dec 9, 2015

Member

That was a mistake. Addressed.

charmtools/build/tactics.py
@@ -253,6 +235,40 @@ def __str__(self):
return "Bind Interface {}".format(self.interface.name)
+class StorageBind(Tactic):
@bcsaller

bcsaller Dec 8, 2015

Contributor

Should this share a baseclass with interface bind and share code. replicating things like sign() happen too much already in the code base. I won't insist, but if you agree that is a good idea we can make an issue.

charmtools/utils.py
@@ -520,3 +520,7 @@ def prefix_lines(lines, lineno):
m=message)
i += 1
return i == 0
+
+
+class BuildError(Exception):
@bcsaller

bcsaller Dec 8, 2015

Contributor

maybe move this to an exceptions module, it isn't really a utility, just a nitpick though

@johnsca

johnsca Dec 9, 2015

Member

I didn't actually mean to move this since I didn't end up using it in tactics.py like I thought I might need to.

@marcoceppi marcoceppi added this to the NEXT milestone Dec 9, 2015

+ return "{}: {}".format(self.__class__.__name__, self.name)
+
+
+class InterfaceBind(DynamicHookBind):
@bcsaller

bcsaller Dec 9, 2015

Contributor

Thank you :)

@johnsca johnsca added the charm-build label Dec 11, 2015

Contributor

chuckbutler commented Dec 11, 2015

From what i see here wrt the storage hook generators, this looks +1 to me, however i haven't verified by attempting to build a charm w/ storage hooks using this branch. YMMV w/ that +1

Member

axw commented Dec 11, 2015

From what i see here wrt the storage hook generators, this looks +1 to me, however i haven't verified by attempting to build a charm w/ storage hooks using this branch. YMMV w/ that +1

I won't have time to test until next week, but if someone wants to test storage, just add the following to a charm's metadata.yaml:

storage:
    whatever:
        type: filesystem

This will create a required filesystem store called "whatever". Deploying without storage constraints should create a directory in the root filesystem that will be used for that store. Just deploying a generated charm that has required storage currently fails, as noted in #70.

Member

johnsca commented Dec 11, 2015

@axw This change is a pre-req for fixing #70. To test this, you will need to update the basic layer to include the template. I will get that proposed now to make this easier to test.

@johnsca johnsca referenced this pull request in juju-solutions/layer-basic Dec 11, 2015

Merged

Refactor bootstrap code and provide hook.template #16

Member

johnsca commented Dec 11, 2015

This can be tested with juju-solutions/reactive-base-layer#16. That PR can be merged before this one. Together, they fix #70

marcoceppi added a commit that referenced this pull request Dec 14, 2015

Merge pull request #75 from johnsca/issue-74
Use hook template from layers instead of embedding

@marcoceppi marcoceppi merged commit 521ea7c into juju:master Dec 14, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment