typesafehub / sbt-conductr

Typesafe ConductR plugin for sbt
Other
29 stars 22 forks source link

BundleId is not correctly display via conductr:info #59

Closed huntc closed 9 years ago

huntc commented 9 years ago

The following output displays for a bundle with the id of 116aa4f196f3ef2b8970b4a4166d0108-a802635856ee251147550871a5f88b46:

ID/BUNDLE/CONF             NAME                          WHERE                   #REP   #STR   #RUN     #CPU     MEM   FSIZE ROLES            
116aa4f/116aa4f/           akkaclusterback-1.0.0         127.0.0.1:9004             1      1      0      1.0   67 MB    5 MB                  

i.e. the 116aa4f is displayed twice.

hseeberger commented 9 years ago

That's not a bug, but another proof that our way to compose a bundle's ID from the bundle digest and the config digest is confusing (@viktorklang). So, in the above case, the bundle digest (i.e. the digest of the bundle alone) is "116aa4f..." and the configuration digest is "a802635..." and we combine those to the bundle ID "116aa4f...-a802635...". Therefore both the (combined) bundle ID and the (sole) bundle digest start with "116aa4f...".

Once again I suggest to get rid of that artificial confusion and create a combined bundle ID which doesn't look like any of the bundle or configuration digest, e.g. via XORing.

Having said this, there still might be a bug here: Why is there no trailing "a802635" after "116aa4f/116aa4f/"?

hseeberger commented 9 years ago

OTOH I can't reproduce this:

Welcome to Scala version 2.10.4 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_40).
Type in expressions to have them evaluated.
Type :help for more information.

scala> import com.typesafe.conductr.client._
import com.typesafe.conductr.client._

scala> import ConductRController._
import ConductRController._

scala> val attr = Attributes(2, 1000, 1000, Set(), "name")
attr: com.typesafe.conductr.client.ConductRController.Attributes = Attributes(2.0,1000,1000,Set(),name)

scala> val info = BundleInfo("116aa4f196f3ef2b8970b4a4166d0108-a802635856ee251147550871a5f88b46", "116aa4f196f3ef2b8970b4a4166d0108", Some("a802635856ee251147550871a5f88b46"), Nil, attr, Set())
info: com.typesafe.conductr.client.ConductRController.BundleInfo = BundleInfo(116aa4f196f3ef2b8970b4a4166d0108-a802635856ee251147550871a5f88b46,116aa4f196f3ef2b8970b4a4166d0108,Some(a802635856ee251147550871a5f88b46),List(),Attributes(2.0,1000,1000,Set(),name),Set())

scala> import com.typesafe.conductr.sbt.console._
import com.typesafe.conductr.sbt.console._

scala> import Column._
import Column._

scala> val id = Id(List(info))
id: com.typesafe.conductr.sbt.console.Column.Id = Id(List(BundleInfo(116aa4f196f3ef2b8970b4a4166d0108-a802635856ee251147550871a5f88b46,116aa4f196f3ef2b8970b4a4166d0108,Some(a802635856ee251147550871a5f88b46),List(),Attributes(2.0,1000,1000,Set(),name),Set())))

scala> id.data
res0: Seq[Seq[String]] = List(List(116aa4f/116aa4f/a802635))
hseeberger commented 9 years ago

@huntc, unless you can provide a reproducible test, I consider this invalid.

viktorklang commented 9 years ago

The big downside with the XORing is that now it becomes harder for the user to know what the combination is (XORing may be better as a handle, but it is arguably more opaque).

So given a bundleid you'd never know if it is a composite id or not, and you'd not know what it was composite of.

hseeberger commented 9 years ago

As you can see, our tools show all relevant information, i.e. compound bundle ID, sole bundle ID (digest) and optional configuration ID.

viktorklang commented 9 years ago

Yes, but when I traverse by bash history and see my conductr commands I've executed via curl, will I be able to find out what bundle I deployed or queried?

hseeberger commented 9 years ago

In my opinion not immediately knowing the meaning of something if better than getting confused/mislead, i.e. consider something for true which is false.

viktorklang commented 9 years ago

On Wed, Apr 8, 2015 at 10:29 AM, Heiko Seeberger notifications@github.com wrote:

In my opinion not immediately knowing the meaning of something if better than getting confused/mislead, i.e. consider something for true which is false.

Do you have an example of that, for my understanding?

— Reply to this email directly or view it on GitHub https://github.com/sbt/sbt-typesafe-conductr/issues/59#issuecomment-90841412 .

Cheers, √

hseeberger commented 9 years ago

See above: Looking at "116aa4f" you don't know whether this is the compound bundle ID or the sole bundle digest.

viktorklang commented 9 years ago

@hseeberger I think that's a fair argument. Perhaps the appropriate solution is to not shorten ids at all, and rely on autocompletion?

huntc commented 9 years ago

Hu Heiko

Let me get you a reproducible test case. Give me an hour or so.

Cheers C

On 8 Apr 2015, at 5:42 pm, Heiko Seeberger notifications@github.com wrote:

That's not a bug, but another proof that our way to compose a bundle's ID from the bundle digest and the config digest is confusing (@viktorklang). So, in the above case, the bundle digest (i.e. the digest of the bundle alone) is "116aa4f..." and the configuration digest is "a802635..." and we combine those to the bundle ID "116aa4f...-a802635...". Therefore both the (combined) bundle ID and the (sole) bundle digest start with "116aa4f...".

Once again I suggest to get rid of that artificial confusion and create a combined bundle ID which doesn't look like any of the bundle or configuration digest, e.g. via XORing.

Having said this, there still might be a bug here: Why is there no trailing "a802635" after "116aa4f/116aa4f/"?

— Reply to this email directly or view it on GitHub.

hseeberger commented 9 years ago

@huntc, don't bother with that now, we can deal with it next week after your vacation!

viktorklang commented 9 years ago

@huntc +1 to what @hseeberger said. Your dedication is exemplary, but now you need to focus on recharging—we have a bit of ways to go still when you get back :)

huntc commented 9 years ago

Its ok re the holiday - it is raining!

Here’s the problem, and it is one due to history. The info command for the plugin displays:

ID/BUNDLE/CONF

We should align this with the Python CLI where ID is displayed only. We should not truncate at all.

We should also have the other columns displayed as per the Python CLI.

There’s also another task to align the commands, but I think that the info command alignment can be covered by this issue.

huntc commented 9 years ago

Actually I think the Python CLI takes the first seven chars of each side of the bundle id.

Let’s just do whatever the Python CLI does. :-)

On 8 Apr 2015, at 8:24 pm, Christopher Hunt huntchr@gmail.com wrote:

We should align this with the Python CLI where ID is displayed only. We should not truncate at all.

hseeberger commented 9 years ago

I agree that the CLI and the plugin should do the same. But for the records, there is no such thing like two sides in general, because if there's no configuration, there's only one side (sigh!).

viktorklang commented 9 years ago

There's always two sides, the choice can be made to make the second part of the default be explicit:

bundlehash-default or bundlehash-bundlehash

huntc commented 9 years ago

Allow me to re-phrase then. :-)

Actually I think the Python CLI takes the first seven chars of each side of the bundle id if there are two sides (delimited with '-'). Otherwise it takes just the first seven chars.

Note that the approach to forming bundleIds hasn't appeared to have confused our albeit small userbase thus far. We should of course keep an eye on that, but I don't think they'll be an issue.

hseeberger commented 9 years ago

Wow, that would result in my confusion being squared. At least.

Ceterum censeo robus esse mutandam.

hseeberger commented 9 years ago

I honestly think the approach is wrong, hence I will end each of my comments with "Ceterum censeo robus esse mutandam."

viktorklang commented 9 years ago

Why would that be confusing? It is using bundle X with bundleXs configuration? :)

On Wed, Apr 8, 2015 at 1:26 PM, Heiko Seeberger notifications@github.com wrote:

Wow, that would result in my confusion being squared. At least.

Ceterum censeo robus esse mutandam.

— Reply to this email directly or view it on GitHub https://github.com/sbt/sbt-typesafe-conductr/issues/59#issuecomment-90884134 .

Cheers, √

huntc commented 9 years ago

Which of course clarifies everything ;-)

hseeberger commented 9 years ago

@viktorklang, there is no such thing like "bundleX's configuration".

huntc commented 9 years ago

@hseeberger points noted though - we should observe our users.

viktorklang commented 9 years ago

Of course there is. Any and all reference.confs + applicaiton.confs and any other configuration embedded within the bundle.

On Wed, Apr 8, 2015 at 1:30 PM, Heiko Seeberger notifications@github.com wrote:

@viktorklang https://github.com/viktorklang, there is no such thing like "bundleX's configuration".

— Reply to this email directly or view it on GitHub https://github.com/sbt/sbt-typesafe-conductr/issues/59#issuecomment-90884727 .

Cheers, √

huntc commented 9 years ago

A configuration file may relate to a bundle, it may also relate to a multitude of bundles - it is up to the user to decide.

hseeberger commented 9 years ago

@viktorklang, this is a misconception. The optional configuration is not a Typesafe-Config one, i.e. no reference.conf or application.conf. It's a script that defines env vars.

huntc commented 9 years ago

There's an example of this in our doc:

http://conductr.typesafe.com/intro/Quickstart.html

Search for "Packaging configuration"

viktorklang commented 9 years ago

@hseeberger "env vars" is an implementation detail, the user value is the configuration it provides to the bundle?

hseeberger commented 9 years ago

@viktorklang, the point I'm making is that there's no default configuration. Configuration is optional. In terms of "implementation detail": It's an Option[_].

viktorklang commented 9 years ago

@hseeberger I think we just have different definitions of "configuration". Do we agree that a bundle most likely contains "configuration"—files that will be read by the program to modify the behavior of said program?

huntc commented 9 years ago

A bundle has configuration. There is configuration for the bundle itself, and there is component oriented configuration. Both of these are within a bundle.

In addition an archive of what we refer to as "optional configuration" can be provided at the point of loading a bundle. The optional configuration can contain both a bundle.conf (for the purposes of overriding the bundle's bundle.conf) and a shell script. The shell script is executed just prior to the bundle's components, and typically exports environment vars. These env vars can be used to override component configuration, which is mostly expressed via Typesafe Config.

'hope that clears things up.

hseeberger commented 9 years ago

@viktorklang. We agree. But there are two flavors. One is the likes of application.conf. The other is the optional configuration that can be applied via Conductr. That one can influence the application.conf ones via env vars, or it can just do something else, because it's just a script.

viktorklang commented 9 years ago

@hseeberger Agreed. So my point is that the composite ID for a deployed bundle without optional (additional) configuration could be bundlehash-none or bundlehash-default or bundlehash- or anything else signalling that it's not like it's different from any other deployment, only that it uses only embedded configuration and no additional configuration. Does that make sense?