Skip to content
This repository was archived by the owner on Dec 11, 2023. It is now read-only.

Error if broker already exists#153

Merged
tzununbekov merged 4 commits intomainfrom
broker-already-exists
Nov 29, 2022
Merged

Error if broker already exists#153
tzununbekov merged 4 commits intomainfrom
broker-already-exists

Conversation

@FranBarrera
Copy link
Copy Markdown
Contributor

@FranBarrera FranBarrera commented Nov 25, 2022

Closes #152

@FranBarrera FranBarrera self-assigned this Nov 25, 2022
@tzununbekov
Copy link
Copy Markdown
Member

tzununbekov commented Nov 25, 2022

It doesn't work if I switch between brokers:

tmctl@broker-already-exists$ tmctl create broker foo
...
2022/11/25 18:20:16 broker: foo already exists

tmctl@broker-already-exists$ tmctl create broker bar
2022/11/25 18:20:26 foo | Updating manifest
2022/11/25 18:20:26 foo | Starting container
---------------
Created object name:	bar
Current broker is set to "bar"
To change the current broker use "tmctl brokers --set <broker name>"
tmctl@broker-already-exists$ tmctl create broker foo
2022/11/25 18:20:33 bar | Updating manifest
2022/11/25 18:20:33 bar | Starting container
---------------
Created object name:	foo
Current broker is set to "foo"
To change the current broker use "tmctl brokers --set <broker name>"

@FranBarrera FranBarrera changed the title Fix broker if already exists Error if broker already exists Nov 25, 2022
@FranBarrera
Copy link
Copy Markdown
Contributor Author

FranBarrera commented Nov 25, 2022

It doesn't work if I switch between brokers:

tmctl@broker-already-exists$ tmctl create broker foo
...
2022/11/25 18:20:16 broker: foo already exists

tmctl@broker-already-exists$ tmctl create broker bar
2022/11/25 18:20:26 foo | Updating manifest
2022/11/25 18:20:26 foo | Starting container
---------------
Created object name:	bar
Current broker is set to "bar"
To change the current broker use "tmctl brokers --set <broker name>"
tmctl@broker-already-exists$ tmctl create broker foo
2022/11/25 18:20:33 bar | Updating manifest
2022/11/25 18:20:33 bar | Starting container
---------------
Created object name:	foo
Current broker is set to "foo"
To change the current broker use "tmctl brokers --set <broker name>"

@tzununbekov But you are not trying to switch between brokers no? You are trying to create the broker foo again, or maybe I miss something.

I've tried to switch between brokers:

⇒  tmctl create broker mybroker
Error: broker: mybroker already exists
Usage:
  tmctl create broker <name> [flags]

Flags:
  -h, --help   help for broker

Global Flags:
      --broker string    Optional broker name.
      --version string   TriggerMesh components version.

2022/11/25 15:24:30 broker: mybroker already exists

⇒  tmctl create broker mybroker2
2022/11/25 15:24:33 mybroker | Updating manifest
2022/11/25 15:24:33 mybroker | Starting container
---------------
Created object name:	mybroker2
Current broker is set to "mybroker2"
To change the current broker use "tmctl brokers --set <broker name>"

⇒  tmctl brokers --set  mybroker2
mybroker
*mybroker2

⇒  tmctl brokers --set  mybroker
*mybroker
mybroker2

@tzununbekov
Copy link
Copy Markdown
Member

tzununbekov commented Nov 25, 2022

Linked issue says:

tmctl should return an error if I try to create a broker with a name that already exists

If I create two brokers, the fix that you are proposing does not follow the behavior you defined in the issue - it does not tell that the broker already exists. Am I missing something?

@FranBarrera
Copy link
Copy Markdown
Contributor Author

FranBarrera commented Nov 25, 2022 •
edited
Loading

Oh I see what you mean, it fails if the context is in a different broker. I'll check it

@tzununbekov
Copy link
Copy Markdown
Member

tzununbekov commented Nov 28, 2022

What is the status of this PR? Can it be reviewed or is it still a draft?

@FranBarrera
Copy link
Copy Markdown
Contributor Author

FranBarrera commented Nov 28, 2022

@tzununbekov It's ready for review

@tzununbekov tzununbekov merged commit 2d494e7 into main Nov 29, 2022
@tzununbekov tzununbekov deleted the broker-already-exists branch November 29, 2022 06:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@odacremolbap odacremolbap Awaiting requested review from odacremolbap

1 more reviewer

@tzununbekov tzununbekov tzununbekov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@FranBarrera FranBarrera

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

tmctl create broker with same name

2 participants

@FranBarrera @tzununbekov