Merge into trunk : 038-upgrade-charm-switch : Code : juju-core

Reviewer	Review Type	Date Requested	Status
Juju Engineering		2013-04-25	Pending
Review via email: mp+160910@code.staging.launchpad.net

Revision history for this message

Dimiter Naydenov (dimitern) wrote on 2013-04-25:

#

Reviewers: mp+160910_code.launchpad.net,

Message:
Please take a look.

Description:
cmd/juju: upgrade-charm --switch support

This implements a new argument to upgrade-charm:
--switch <charm-url>. The passed URL is inferred
to get the complete URL and used instead of the
service's newest charm url revision.

There are a few related sanity checks that need
to be implemented as well (in follow-ups), so we
can better check charm compatibility before upgrading.
Fixes bug #1040203.

https://code.launchpad.net/~dimitern/juju-core/038-upgrade-charm-switch/+merge/160910

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/8540050/

Affected files:
   A [revision details]
   M cmd/juju/upgradecharm.go
   M cmd/juju/upgradecharm_test.go
   M testing/charm.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:
<email address hidden>
+New revision:
<email address hidden>

Index: testing/charm.go
=== modified file 'testing/charm.go'
--- testing/charm.go 2013-02-11 05:54:45 +0000
+++ testing/charm.go 2013-04-25 14:27:20 +0000
@@ -54,6 +54,15 @@
return clone(dst, r.DirPath(name))
}

+// RenamedClonedDirPath returns the path to a new copy of the default
+// charm directory named name, but renames it to newName.
+func (r *Repo) RenamedClonedDirPath(dst, name, newName string) string {
+ newDst := clone(dst, r.DirPath(name))
+ renamedDst := filepath.Join(filepath.Dir(newDst), newName)
+ check(exec.Command("mv", newDst, renamedDst).Run())
+ return renamedDst
+}
+
  // ClonedDir returns an actual charm.Dir based on a new copy of the charm
directory
  // named name, in the directory dst.
  func (r *Repo) ClonedDir(dst, name string) *charm.Dir {

Index: cmd/juju/upgradecharm.go
=== modified file 'cmd/juju/upgradecharm.go'
--- cmd/juju/upgradecharm.go 2013-04-25 10:46:51 +0000
+++ cmd/juju/upgradecharm.go 2013-04-25 14:27:20 +0000
@@ -17,6 +17,7 @@
   ServiceName string
   Force bool
   RepoPath string // defaults to JUJU_REPOSITORY
+ SwitchURL string
  }

  const upgradeCharmDoc = `
@@ -32,6 +33,10 @@
  author working on a single client machine; use of local repositories from
  multiple clients is not supported and may lead to confusing behaviour.

+To manually specify the charm URL to upgrade to, use the --switch argument.
+It will be used instead of the service's current charm newest revision.
+Note that the given charm must be compatible with the current one.
+
  Use of the --force flag is not generally recommended; units upgraded while
in
  an error state will not have upgrade-charm hooks executed, and may cause
  unexpected behavior.
@@ -50,6 +55,7 @@
   c.EnvCommandBase.SetFlags(f)
   f.BoolVar(&c.Force, "force", false, "upgrade all units immediately, even
if in error state")
   f.StringVar(&c.RepoPath, "repository",
os.Getenv("JUJU_REPOSITORY"), "local charm repository path")
+ f.StringVar(&c.SwitchURL, "switch", "", "charm URL to upgrade to")
  }

func (c *Up...

Reviewers: mp+160910_code.launchpad.net,

Message:
Please take a look.

Description:
cmd/juju: upgrade-charm --switch support

This implements a new argument to upgrade-charm:
--switch <charm-url>. The passed URL is inferred
to get the complete URL and used instead of the
service's newest charm url revision.

There are a few related sanity checks that need
to be implemented as well (in follow-ups), so we
can better check charm compatibility before upgrading.
Fixes bug #1040203.

https://code.launchpad.net/~dimitern/juju-core/038-upgrade-charm-switch/+merge/160910

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/8540050/

Affected files:
   A [revision details]
   M cmd/juju/upgradecharm.go
   M cmd/juju/upgradecharm_test.go
   M testing/charm.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details]	2012-01-01 00:00:00 +0000
+++ [revision details]	2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision:  
dimiter.naydenov@canonical.com-20130425104651-fgtnvua8yuxq0nl9
+New revision:  
dimiter.naydenov@canonical.com-20130425142720-ha499hiz814v02ju

Index: testing/charm.go
=== modified file 'testing/charm.go'
--- testing/charm.go	2013-02-11 05:54:45 +0000
+++ testing/charm.go	2013-04-25 14:27:20 +0000
@@ -54,6 +54,15 @@
  	return clone(dst, r.DirPath(name))
  }

+// RenamedClonedDirPath returns the path to a new copy of the default
+// charm directory named name, but renames it to newName.
+func (r *Repo) RenamedClonedDirPath(dst, name, newName string) string {
+	newDst := clone(dst, r.DirPath(name))
+	renamedDst := filepath.Join(filepath.Dir(newDst), newName)
+	check(exec.Command("mv", newDst, renamedDst).Run())
+	return renamedDst
+}
+
  // ClonedDir returns an actual charm.Dir based on a new copy of the charm  
directory
  // named name, in the directory dst.
  func (r *Repo) ClonedDir(dst, name string) *charm.Dir {

Index: cmd/juju/upgradecharm.go
=== modified file 'cmd/juju/upgradecharm.go'
--- cmd/juju/upgradecharm.go	2013-04-25 10:46:51 +0000
+++ cmd/juju/upgradecharm.go	2013-04-25 14:27:20 +0000
@@ -17,6 +17,7 @@
  	ServiceName string
  	Force       bool
  	RepoPath    string // defaults to JUJU_REPOSITORY
+	SwitchURL   string
  }

const upgradeCharmDoc = `
@@ -32,6 +33,10 @@
  author working on a single client machine; use of local repositories from
  multiple clients is not supported and may lead to confusing behaviour.

+To manually specify the charm URL to upgrade to, use the --switch argument.
+It will be used instead of the service's current charm newest revision.
+Note that the given charm must be compatible with the current one.
+
  Use of the --force flag is not generally recommended; units upgraded while  
in
  an error state will not have upgrade-charm hooks executed, and may cause
  unexpected behavior.
@@ -50,6 +55,7 @@
  	c.EnvCommandBase.SetFlags(f)
  	f.BoolVar(&c.Force, "force", false, "upgrade all units immediately, even  
if in error state")
  	f.StringVar(&c.RepoPath, "repository",  
os.Getenv("JUJU_REPOSITORY"), "local charm repository path")
+	f.StringVar(&c.SwitchURL, "switch", "", "charm URL to upgrade to")
  }

func (c *UpgradeCharmCommand) Init(args []string) error {
@@ -64,7 +70,6 @@
  	default:
  		return cmd.CheckEmpty(args[1:])
  	}
-	// TODO(dimitern): add the other flags --switch and --revision.
  	return nil
  }

@@ -80,7 +85,20 @@
  	if err != nil {
  		return err
  	}
-	curl, _ := service.CharmURL()
+	var curl *charm.URL
+	if c.SwitchURL != "" {
+		var err error
+		conf, err := conn.State.EnvironConfig()
+		if err != nil {
+			return err
+		}
+		curl, err = charm.InferURL(c.SwitchURL, conf.DefaultSeries())
+		if err != nil {
+			return err
+		}
+	} else {
+		curl, _ = service.CharmURL()
+	}
  	repo, err := charm.InferRepository(curl, ctx.AbsPath(c.RepoPath))
  	if err != nil {
  		return err

Index: cmd/juju/upgradecharm_test.go
=== modified file 'cmd/juju/upgradecharm_test.go'
--- cmd/juju/upgradecharm_test.go	2013-03-27 15:42:27 +0000
+++ cmd/juju/upgradecharm_test.go	2013-04-25 14:27:20 +0000
@@ -58,6 +58,18 @@
  	c.Assert(err, ErrorMatches, `already running latest  
charm "local:precise/riak-7"`)
  }

+func (s *UpgradeCharmErrorsSuite) TestInvalidSwitchURL(c *C) {
+	testing.Charms.ClonedDirPath(s.seriesPath, "riak")
+	err := runDeploy(c, "local:riak", "riak")
+	c.Assert(err, IsNil)
+
+	err = runUpgradeCharm(c, "riak", "--switch=blah")
+	c.Assert(err, ErrorMatches, "charm not found: cs:precise/blah")
+	err = runUpgradeCharm(c, "riak", "--switch=cs:missing/one-1")
+	c.Assert(err, ErrorMatches, "charm not found: cs:missing/one")
+	// TODO(dimitern): add tests with incompatible charms
+}
+
  type UpgradeCharmSuccessSuite struct {
  	repoSuite
  	path string
@@ -79,7 +91,7 @@
  	c.Assert(forced, Equals, false)
  }

-func (s *UpgradeCharmSuccessSuite) assertUpgraded(c *C, revision int,  
forced bool) {
+func (s *UpgradeCharmSuccessSuite) assertUpgraded(c *C, revision int,  
forced bool, curl *charm.URL) {
  	err := s.riak.Refresh()
  	c.Assert(err, IsNil)
  	ch, force, err := s.riak.Charm()
@@ -87,10 +99,13 @@
  	c.Assert(ch.Revision(), Equals, revision)
  	c.Assert(force, Equals, forced)
  	s.assertCharmUploaded(c, ch.URL())
+	if curl != nil {
+		c.Assert(ch.URL().String(), Equals, curl.String())
+	}
  }

-func (s *UpgradeCharmSuccessSuite) assertLocalRevision(c *C, revision int)  
{
-	dir, err := charm.ReadDir(s.path)
+func (s *UpgradeCharmSuccessSuite) assertLocalRevision(c *C, revision int,  
path string) {
+	dir, err := charm.ReadDir(path)
  	c.Assert(err, IsNil)
  	c.Assert(dir.Revision(), Equals, revision)
  }
@@ -98,8 +113,8 @@
  func (s *UpgradeCharmSuccessSuite) TestBumpsRevisionWhenNecessary(c *C) {
  	err := runUpgradeCharm(c, "riak")
  	c.Assert(err, IsNil)
-	s.assertUpgraded(c, 8, false)
-	s.assertLocalRevision(c, 8)
+	s.assertUpgraded(c, 8, false, nil)
+	s.assertLocalRevision(c, 8, s.path)
  }

func (s *UpgradeCharmSuccessSuite)  
TestDoesntBumpRevisionWhenNotNecessary(c *C) {
@@ -110,8 +125,8 @@

err = runUpgradeCharm(c, "riak")
  	c.Assert(err, IsNil)
-	s.assertUpgraded(c, 42, false)
-	s.assertLocalRevision(c, 42)
+	s.assertUpgraded(c, 42, false, nil)
+	s.assertLocalRevision(c, 42, s.path)
  }

func (s *UpgradeCharmSuccessSuite) TestUpgradesWithBundle(c *C) {
@@ -128,13 +143,38 @@

err = runUpgradeCharm(c, "riak")
  	c.Assert(err, IsNil)
-	s.assertUpgraded(c, 42, false)
-	s.assertLocalRevision(c, 7)
+	s.assertUpgraded(c, 42, false, nil)
+	s.assertLocalRevision(c, 7, s.path)
  }

func (s *UpgradeCharmSuccessSuite) TestForcedUpgrade(c *C) {
  	err := runUpgradeCharm(c, "riak", "--force")
  	c.Assert(err, IsNil)
-	s.assertUpgraded(c, 8, true)
-	s.assertLocalRevision(c, 8)
+	s.assertUpgraded(c, 8, true, nil)
+	s.assertLocalRevision(c, 8, s.path)
+}
+
+var myriakMeta = []byte(`
+name: myriak
+summary: "K/V storage engine"
+description: "Scalable K/V Store in Erlang with Clocks :-)"
+provides:
+  endpoint:
+    interface: http
+  admin:
+    interface: http
+peers:
+  ring:
+    interface: riak
+`)
+
+func (s *UpgradeCharmSuccessSuite) TestSwitch(c *C) {
+	myriakPath :=  
testing.Charms.RenamedClonedDirPath(s.seriesPath, "riak", "myriak")
+	err := ioutil.WriteFile(path.Join(myriakPath, "metadata.yaml"),  
myriakMeta, 0644)
+	c.Assert(err, IsNil)
+
+	err = runUpgradeCharm(c, "riak", "--switch=local:myriak")
+	c.Assert(err, IsNil)
+	s.assertUpgraded(c, 7, false,  
charm.MustParseURL("local:precise/myriak-7"))
+	s.assertLocalRevision(c, 7, myriakPath)
  }

Revision history for this message

Roger Peppe (rogpeppe) wrote on 2013-04-25:

#

good direction, but does not LGTM quite yet - some logic i don't think
is quite right, as outlined below.

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go
File cmd/juju/upgradecharm.go (right):

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#newcode38
cmd/juju/upgradecharm.go:38: Note that the given charm must be
compatible with the current one.
Perhaps say what "compatible" means in this context in a concise and
easy to understand way (no need to go into all the details, but an
overview would be good so the user knows what they're up against)

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#newcode106
cmd/juju/upgradecharm.go:106: rev, err := repo.Latest(curl)
i think this is wrong in the context of SwitchURL.

i think the Latest and bump-revision logic should only be triggered when
the charm url does not have a revision number specified.

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm_test.go
File cmd/juju/upgradecharm_test.go (right):

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm_test.go#newcode179
cmd/juju/upgradecharm_test.go:179: s.assertLocalRevision(c, 7,
myriakPath)
also test switching from one cs: charm to another?
and with a revision number explicitly specified.

https://codereview.appspot.com/8540050/diff/1/testing/charm.go
File testing/charm.go (right):

https://codereview.appspot.com/8540050/diff/1/testing/charm.go#newcode62
testing/charm.go:62: check(exec.Command("mv", newDst, renamedDst).Run())
os.Rename(newDst, renamedDst) ?

https://codereview.appspot.com/8540050/

Revision history for this message

William Reade (fwereade) wrote on 2013-04-25:

#

a few thoughts

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go
File cmd/juju/upgradecharm.go (left):

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#oldcode67
cmd/juju/upgradecharm.go:67: // TODO(dimitern): add the other flags
--switch and --revision.
I don't see an implementation of --revision, and I think it'd be quite
easy to sneak it into this CL ;)

(consider that --switch and --revision could in theory conflict, so it
might be good to barf here if they're both set)

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go
File cmd/juju/upgradecharm.go (right):

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#newcode38
cmd/juju/upgradecharm.go:38: Note that the given charm must be
compatible with the current one.
On 2013/04/25 14:43:52, rog wrote:
> Perhaps say what "compatible" means in this context in a concise and
easy to
> understand way (no need to go into all the details, but an overview
would be
> good so the user knows what they're up against)

and underscore that it's dangerous -- we can only check so much
compatibility :)

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#newcode100
cmd/juju/upgradecharm.go:100: curl, _ = service.CharmURL()
wrt rog's comment below, curl = curl.WithRevision(-1)

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#newcode106
cmd/juju/upgradecharm.go:106: rev, err := repo.Latest(curl)
On 2013/04/25 14:43:52, rog wrote:
> i think this is wrong in the context of SwitchURL.

> i think the Latest and bump-revision logic should only be triggered
when the
> charm url does not have a revision number specified.

+1

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm_test.go
File cmd/juju/upgradecharm_test.go (right):

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm_test.go#newcode94
cmd/juju/upgradecharm_test.go:94: func (s *UpgradeCharmSuccessSuite)
assertUpgraded(c *C, revision int, forced bool, curl *charm.URL) {
I'd rather return the charm url and assert it in the places it's needed,
I think

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm_test.go#newcode179
cmd/juju/upgradecharm_test.go:179: s.assertLocalRevision(c, 7,
myriakPath)
On 2013/04/25 14:43:52, rog wrote:
> also test switching from one cs: charm to another?
> and with a revision number explicitly specified.

We have thus far gone with the shared agreement that if it works with
one repo (local), we can expect it to work with the other. Hmm, can we
now fake out the charm store? I think we can. If so, definite +1 on
testing this in a wider range of scenarios.

https://codereview.appspot.com/8540050/diff/1/testing/charm.go
File testing/charm.go (right):

https://codereview.appspot.com/8540050/diff/1/testing/charm.go#newcode62
testing/charm.go:62: check(exec.Command("mv", newDst, renamedDst).Run())
On 2013/04/25 14:43:52, rog wrote:
> os.Rename(newDst, renamedDst) ?

+1

https://codereview.appspot.com/8540050/

a few thoughts

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go
File cmd/juju/upgradecharm.go (left):

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#oldcode67
cmd/juju/upgradecharm.go:67: // TODO(dimitern): add the other flags
--switch and --revision.
I don't see an implementation of --revision, and I think it'd be quite
easy to sneak it into this CL ;)

(consider that --switch and --revision could in theory conflict, so it
might be good to barf here if they're both set)

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go
File cmd/juju/upgradecharm.go (right):

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#newcode38
cmd/juju/upgradecharm.go:38: Note that the given charm must be
compatible with the current one.
On 2013/04/25 14:43:52, rog wrote:
> Perhaps say what "compatible" means in this context in a concise and
easy to
> understand way (no need to go into all the details, but an overview
would be
> good so the user knows what they're up against)

and underscore that it's dangerous -- we can only check so much
compatibility :)

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#newcode100
cmd/juju/upgradecharm.go:100: curl, _ = service.CharmURL()
wrt rog's comment below, curl = curl.WithRevision(-1)

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#newcode106
cmd/juju/upgradecharm.go:106: rev, err := repo.Latest(curl)
On 2013/04/25 14:43:52, rog wrote:
> i think this is wrong in the context of SwitchURL.

> i think the Latest and bump-revision logic should only be triggered
when the
> charm url does not have a revision number specified.

+1

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm_test.go
File cmd/juju/upgradecharm_test.go (right):

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm_test.go#newcode94
cmd/juju/upgradecharm_test.go:94: func (s *UpgradeCharmSuccessSuite)
assertUpgraded(c *C, revision int, forced bool, curl *charm.URL) {
I'd rather return the charm url and assert it in the places it's needed,
I think

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm_test.go#newcode179
cmd/juju/upgradecharm_test.go:179: s.assertLocalRevision(c, 7,
myriakPath)
On 2013/04/25 14:43:52, rog wrote:
> also test switching from one cs: charm to another?
> and with a revision number explicitly specified.

We have thus far gone with the shared agreement that if it works with
one repo (local), we can expect it to work with the other. Hmm, can we
now fake out the charm store? I think we can. If so, definite +1 on
testing this in a wider range of scenarios.

https://codereview.appspot.com/8540050/diff/1/testing/charm.go
File testing/charm.go (right):

https://codereview.appspot.com/8540050/diff/1/testing/charm.go#newcode62
testing/charm.go:62: check(exec.Command("mv", newDst, renamedDst).Run())
On 2013/04/25 14:43:52, rog wrote:
> os.Rename(newDst, renamedDst) ?

+1

https://codereview.appspot.com/8540050/

Revision history for this message

Dimiter Naydenov (dimitern) wrote on 2013-04-25:

#

Download full text (4.2 KiB)

Nothing changed yet, I just need to make sure I get the suggestions
first.

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go
File cmd/juju/upgradecharm.go (left):

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#oldcode67
cmd/juju/upgradecharm.go:67: // TODO(dimitern): add the other flags
--switch and --revision.
On 2013/04/25 15:09:06, fwereade wrote:
> I don't see an implementation of --revision, and I think it'd be quite
easy to
> sneak it into this CL ;)

> (consider that --switch and --revision could in theory conflict, so it
might be
> good to barf here if they're both set)

So --revision is an int and forces the revision as specified, but only
when --switch is not present, otherwise - barf?

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go
File cmd/juju/upgradecharm.go (right):

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#newcode38
cmd/juju/upgradecharm.go:38: Note that the given charm must be
compatible with the current one.
On 2013/04/25 15:09:06, fwereade wrote:
> On 2013/04/25 14:43:52, rog wrote:
> > Perhaps say what "compatible" means in this context in a concise and
easy to
> > understand way (no need to go into all the details, but an overview
would be
> > good so the user knows what they're up against)

> and underscore that it's dangerous -- we can only check so much
compatibility :)

I'd appreciate suggestions with actual text, since probably both of you
can devise a better sentence than me :)

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#newcode100
cmd/juju/upgradecharm.go:100: curl, _ = service.CharmURL()
On 2013/04/25 15:09:06, fwereade wrote:
> wrt rog's comment below, curl = curl.WithRevision(-1)

When --switch is given, the revision shouldn't be bumped, and the code
below does not apply, right?

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm.go#newcode106
cmd/juju/upgradecharm.go:106: rev, err := repo.Latest(curl)
On 2013/04/25 15:09:06, fwereade wrote:
> On 2013/04/25 14:43:52, rog wrote:
> > i think this is wrong in the context of SwitchURL.
> >
> > i think the Latest and bump-revision logic should only be triggered
when the
> > charm url does not have a revision number specified.

> +1

See the question above.

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm_test.go
File cmd/juju/upgradecharm_test.go (right):

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm_test.go#newcode94
cmd/juju/upgradecharm_test.go:94: func (s *UpgradeCharmSuccessSuite)
assertUpgraded(c *C, revision int, forced bool, curl *charm.URL) {
On 2013/04/25 15:09:06, fwereade wrote:
> I'd rather return the charm url and assert it in the places it's
needed, I think

Good point, will do.

https://codereview.appspot.com/8540050/diff/1/cmd/juju/upgradecharm_test.go#newcode107
cmd/juju/upgradecharm_test.go:107: func (s *UpgradeCharmSuccessSuite)
assertLocalRevision(c *C, revision int, path string) {
On 2013/04/25 14:40:06, TheMue wrote:
> Maybe I'm only blind or it is planned for later, but where are you
using this
> method with a different path than s.path?

Only in the last t...