1139279 - taskcluster-client: Allow specifying connection string in pulse listener instead of user/pass

Status: RESOLVED FIXED

(Taskcluster :: General, defect)

Description

[James Lal [:lightsofapollo]]

In some situations it would be nice to just specify connection string (particularly when testing amqp stuff locally). We currently have the hostname option but I don't really want to specify three configs. This is somewhat more complicated in my case as I want a mix of pulse and local docker amqp setup.

Comment 1

[James Lal [:lightsofapollo]]

Attachment: https://github.com/taskcluster/taskcluster-client/pull/14/files

Comment 2

[Jonas Finnemann Jensen (:jonasfj)]

Attachment: Github PR with explicit namespace

Basically be explicite about the `namespace`. And don't parse it if loading from connectionString.

This is pretty much details. I'll think this in when we write pulse client into something more generic... It's far down my todo, but it's there.

Comment 3

[Jonas Finnemann Jensen (:jonasfj)]

Comment on attachment 8572478 [details]
https://github.com/taskcluster/taskcluster-client/pull/14/files

We could totally merge this, I'm not strongly opposed, just suggesting an alternative.

As mentioned before I'm not even sure about how this will look in the future.

Btw, I use real when testing locally, there is a reason why QueueEvents let's
you overwrite `exchangePrefix`.
Ie:
new taskcluster.QueueEvents({exchangePrefix: 'exchange/my-fake-queue-user/'});

If I'm creating a fake queue with pulse user "my-fake-queue-user".

Comment 4

[Jonas Finnemann Jensen (:jonasfj)]

Note, to self:
"exchangePrefix" should probably be turned into "namespace", and not include the "exchange/" part, but have that automatically prefixed.

Comment 5

[James Lal [:lightsofapollo]]

Comment on attachment 8573034 [details] [review]
Github PR with explicit namespace

I am fine with this approach too but in general I don't really like object properties in configs which either override each other or are mutually exclusive (which is why I chose to clobber it with a string OR object).

Another potential thing would be to move "namespace" out of credentials ?

Comment 6

[Jonas Finnemann Jensen (:jonasfj)]

> Another potential thing would be to move "namespace" out of credentials ?
Namespace is inherently coupled with credentials in pulse. And ought to be in any AMQP deployment.

I don't like properties that overwrite, each other either... being mutually exclusive I can better live with, then we can raise an error instead of having weird behaviour.

Will check that username/password/hostname isn't provided along with `connectionString` and land this.

Comment 7

[Jonas Finnemann Jensen (:jonasfj)]

Merged... Will publish nowish, as we change the username property to namespace, we'll likley need to a major version number bump...

Comment 8

[Pete Moore [:pmoore][:pete]]

Resetting Version and Target Milestone that accidentally got changed...