Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

self linking of the component should not be there using odo link #1052

Closed
mohitsuman opened this issue Nov 29, 2018 · 15 comments
Closed

self linking of the component should not be there using odo link #1052

mohitsuman opened this issue Nov 29, 2018 · 15 comments
Labels
area/binding Issues or PRs related to `odo add/delete binding *` commands or Service Binding Operator estimated-size/XS (1-5) Rough sizing for Epics. Less than one sprint of work for one person, but smaller than S. kind/bug Categorizes issue or PR as related to a bug. priority/Low Nice to have issue. It's not immediately on the project roadmap to get it done.

Comments

@mohitsuman
Copy link

mohitsuman commented Nov 29, 2018

[kind/bug]

What versions of software are you using?

  • Operating System: macOS
  • Output of odo version: odo v0.0.16 (82968cb)

How did you run odo exactly?

Running odo from the binary.

Actual behavior

If I do odo link on the component and pass the same component name in the --component parameter, it works.

e.g. odo link nodejs --component nodejs

Expected behavior

We should handle this condition and make sure that self linking of the component is not allowed or a warning is shown.

@geoand
Copy link
Contributor

geoand commented Nov 30, 2018

Hi @mohitsuman

This indeed should not be possible, but I am curious to what led you to executing such a command?
Where you just trying to brake stuff or was something in the command names or help not clear?

Thanks

@mohitsuman
Copy link
Author

Hi @geoand ,

We are working on OpenShift extension which uses odo under the hood. Now in the VSCode UI, when the user is given a list of components to link, it can be have the same component in the list and running the odo link succeeded into that.

We for now are handling this condition on UI and not showing the same component in the list to select, but it reminded us that odo link should also have a check for this.

@geoand
Copy link
Contributor

geoand commented Nov 30, 2018

@mohitsuman Thanks for the insight!

@jorgemoralespou
Copy link
Contributor

I think this is something the CLI should probably not control. In this case, I think this is more a feature for the VSCode plugin and I think it solved the right way, by filtering the component there.

In any case, there's also the possibility to have the same component name in a different application, so if at any point, this issue is solved in odo, all different possibilities should be considered, which means that --app is not provided or if that it's provided it's not the same as current.

@kadel kadel added state/Ready priority/Low Nice to have issue. It's not immediately on the project roadmap to get it done. labels Dec 3, 2018
@kadel
Copy link
Member

kadel commented Jan 22, 2019

I think this is something the CLI should probably not control. In this case, I think this is more a feature for the VSCode plugin and I think it solved the right way, by filtering the component there.

In any case, there's also the possibility to have the same component name in a different application, so if at any point, this issue is solved in odo, all different possibilities should be considered, which means that --app is not provided or if that it's provided it's not the same as current.

Shouldn't odo check that user is not doing something stupid on invalid?
I think that self-linking should be something that odo should catch and error out.

Of course, it has to be smart enough to recognize that you are linking to a different component with the same name and allow that.

@geoand
Copy link
Contributor

geoand commented Jan 22, 2019

I think this is something the CLI should probably not control. In this case, I think this is more a feature for the VSCode plugin and I think it solved the right way, by filtering the component there.
In any case, there's also the possibility to have the same component name in a different application, so if at any point, this issue is solved in odo, all different possibilities should be considered, which means that --app is not provided or if that it's provided it's not the same as current.

Shouldn't odo check that user is not doing something stupid on invalid?
I think that self-linking should be something that odo should catch and error out.

Of course, it has to be smart enough to recognize that you are linking to a different component with the same name and allow that.

I agree that odo should try and prevent things that don't make sense

@girishramnani girishramnani added area/binding Issues or PRs related to `odo add/delete binding *` commands or Service Binding Operator kind/bug Categorizes issue or PR as related to a bug. labels Jan 2, 2020
@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 14, 2020
@openshift-bot
Copy link

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

@openshift-ci-robot openshift-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 13, 2020
@mohitsuman
Copy link
Author

@dharmit any update for this ?

@dharmit
Copy link
Member

dharmit commented Jun 19, 2020

@mohitsuman I'm adding this to our team discussion. Mainly because this is something we need to discuss for odo v2 as well even though this issue was opened for v1.

@dharmit dharmit removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jun 19, 2020
@mohitsuman
Copy link
Author

@dharmit sure thing, let us know the discussion conclusion.

@openshift-bot
Copy link

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

@openshift-ci-robot openshift-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 22, 2020
@dharmit
Copy link
Member

dharmit commented Nov 10, 2020

@mohitsuman I'm closing this one since it's being tracked as an acceptance criterion on #3423.

/close

@openshift-ci-robot
Copy link
Collaborator

@dharmit: Closing this issue.

In response to this:

@mohitsuman I'm closing this one since it's being tracked as an acceptance criterion on #3423.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kadel kadel removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 8, 2021
@dharmit
Copy link
Member

dharmit commented Jun 16, 2021

@mohitsuman this has been fixed by #4777.

@rm3l rm3l added the estimated-size/XS (1-5) Rough sizing for Epics. Less than one sprint of work for one person, but smaller than S. label Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/binding Issues or PRs related to `odo add/delete binding *` commands or Service Binding Operator estimated-size/XS (1-5) Rough sizing for Epics. Less than one sprint of work for one person, but smaller than S. kind/bug Categorizes issue or PR as related to a bug. priority/Low Nice to have issue. It's not immediately on the project roadmap to get it done.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

9 participants