implement parsing of Min Juju Version #176

Closed
wants to merge 8 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

natefinch commented Nov 21, 2015

This adds a very simple min-juju-version key into the metadata.yaml that accepts the standard 1.25.0 style version number for comparison to the juju server version (though the comparison is done in core). This PR depends on juju/version#1 which moves the juju version parsing code out of juju/juju so that the charm code can use it.

charm_test.go
+
+func (s *CharmSuite) TestIsMinVersionError(c *gc.C) {
+ err := charm.MinVersionError()
+ c.Assert(charm.IsMinVersionError(err), jc.IsTrue)
@fwereade

fwereade Nov 24, 2015

Contributor

nice to make these Checks so they don't stop the test

meta.go
+ Subordinate bool `yaml:"subordinate,omitempty"`
+ Series []string `yaml:"series,omitempty"`
+ Terms []string `yaml:"terms,omitempty"`
+ MinJujuVersion *version.Number `yaml:"min-juju-version,omitempty"`
@fwereade

fwereade Nov 24, 2015

Contributor

not really clear why this is a pointer -- surely the zero version.Number is perfectly suitable, and less likely to induce potential panics when not initialized...

@natefinch

natefinch Nov 25, 2015

Contributor

Oh yeah, totally. Wasn't thinking.

meta.go
@@ -277,9 +279,20 @@ func ReadMeta(r io.Reader) (meta *Meta, err error) {
meta.Series = parseStringList(m["series"])
meta.Storage = parseStorage(m["storage"])
meta.PayloadClasses = parsePayloadClasses(m["payloads"])
+
+ if v, ok := m["min-juju-version"].(string); ok {
@fwereade

fwereade Nov 24, 2015

Contributor

surely missing is fine, and valid is fine, but I think invalid data should be considered an error

@natefinch

natefinch Nov 25, 2015

Contributor

good point.

charm.go
+// CheckMinVersion reports an error that reports true from IsMinVersionErr if
+// the given charm has a minimum juju version specified that is higher than the
+// given juju version number.
+func CheckMinVersion(ch Charm, jujuVersion version.Number) error {
@fwereade

fwereade Nov 24, 2015

Contributor

this doesn't quite feel like the right place for the error -- and the reference to an "environment" below makes me doubly sure (charm shouldn't need to reference the concept at all). Please move this stuff inside juju/juju

Contributor

natefinch commented Nov 27, 2015

moving to a feature branch

@natefinch natefinch closed this Nov 27, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment