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

Support specifying confusing instance names #763

Closed
DifferentialOrange opened this issue Apr 14, 2023 · 0 comments · Fixed by #765
Closed

Support specifying confusing instance names #763

DifferentialOrange opened this issue Apr 14, 2023 · 0 comments · Fixed by #765
Assignees
Labels
bug Something isn't working customer

Comments

@DifferentialOrange
Copy link
Member

1. Application name as instance name

Now we don't support instance names which coincide with application names:

cat ./instances.yml 
---
myapp.myapp:
  advertise_uri: localhost:3301
  http_port: 8081
cartridge start -d
   • myapp.myapp... OK
cartridge status
   • myapp.myapp: RUNNING
cartridge status myapp
   ⨯ Application name is specified. Please, specify instance name(s)

2. Instance name with dots

The reason is this code in GetInstancesFromArgs method:

if instanceName == projectName {
return nil, fmt.Errorf(appNameSpecifiedError)
}

We also do not support instance names with dots:

cat ./instances.yml 
---
myapp.my.app:
  advertise_uri: localhost:3301
  http_port: 8081
cartridge start -d
   • myapp.my.app... OK
cartridge status
   • myapp.my.app: RUNNING
cartridge status my.app
   ⨯ [APP_NAME].INSTANCE_NAME is specified. Please, specify instance name(s)

The reason is this code in GetInstancesFromArgs method:

parts := strings.SplitN(instanceName, ".", 2)
if len(parts) > 1 {
return nil, fmt.Errorf(instanceIDSpecified)
}

3. Specifying an instance name multiple times

GetInstancesFromArgs also doesn't allow to specify an instance multiple times:

if _, found := foundInstances[instanceName]; found {
return nil, fmt.Errorf("Duplicate instance name specified: %s", instanceName)
}


Proposed solution

The reason of the issues 1 and 2 is the desire to help a user if they specified application_name or application_name.instance_name instead of instance_name. It results in inability to use some instance names in some commands which are (likely) supported by any other command. We need to

  1. allow a user to use such names
  2. after verifying that any other command is fine with that.

To preserve existing helpful error messages in case user really specified application_name or application_name.instance_name instead of instance_name (which is rather often), we'll need to rework our approach. If user had provided an invalid info, the command should fail. And if the command had failed,

  1. we additionally check if they had specified application_name or application_name.instance_name instead of instance_name and, if they are, add this info to error message.

Regarding issue 3, we may simply

  1. start to skip duplicate names instead of throwing an error,

but, since there are no such request from customers yet, we may ignore this one for now.

Based on https://jira.vk.team/browse/TNT-765

@DifferentialOrange DifferentialOrange added bug Something isn't working customer teamE labels Apr 14, 2023
GRISHNOV added a commit that referenced this issue Apr 19, 2023
GRISHNOV added a commit that referenced this issue Apr 19, 2023
GRISHNOV added a commit that referenced this issue Apr 19, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
GRISHNOV added a commit that referenced this issue Apr 19, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
GRISHNOV added a commit that referenced this issue Apr 19, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
GRISHNOV added a commit that referenced this issue Apr 20, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
GRISHNOV added a commit that referenced this issue Apr 20, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
GRISHNOV added a commit that referenced this issue Apr 20, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
GRISHNOV added a commit that referenced this issue Apr 20, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
GRISHNOV added a commit that referenced this issue Apr 21, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
GRISHNOV added a commit that referenced this issue Apr 21, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
GRISHNOV added a commit that referenced this issue Apr 24, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
GRISHNOV added a commit that referenced this issue Apr 24, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
GRISHNOV added a commit that referenced this issue Apr 24, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
DifferentialOrange pushed a commit that referenced this issue Apr 24, 2023
Added instance name support for cases:
* Application name as instance name
* Instance name with dots

Closes #763
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working customer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants