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

Equality check and/or introspection of objects changes choice #139

Open
ajbalogh opened this issue Aug 26, 2021 · 3 comments · May be fixed by #394
Open

Equality check and/or introspection of objects changes choice #139

ajbalogh opened this issue Aug 26, 2021 · 3 comments · May be fixed by #394
Assignees
Labels
bug Something isn't working priority High Priority ux Impacts UX
Milestone

Comments

@ajbalogh
Copy link
Contributor

The following sample code changes the choice.
Choice should only change on setattr not on getattr.

tcp.dst_port.value = 80
# the following should throw an exception
if tcp.dst_port.values[0] == 0:
   pass
@ajbalogh ajbalogh added bug Something isn't working py ux sdk gen labels Aug 26, 2021
@Rangababu-R
Copy link
Contributor

@alakendu is this issue still need to be fixed?

@alakendu
Copy link
Contributor

No, we put it in a backlog.

@ashutshkumr ashutshkumr added this to the v1.0.0 milestone Jan 27, 2023
@Vibaswan Vibaswan assigned Vibaswan and unassigned alakendu Feb 7, 2023
@Vibaswan Vibaswan linked a pull request Feb 8, 2023 that will close this issue
@Vibaswan
Copy link
Collaborator

Vibaswan commented Feb 13, 2023

Proposal

Information on choice

Choice is actually a property in the model which acts as a discriminatory field for the users . A choice has multiple options to choose from for the user. User sets the choice according to their use-cases.
The options under a choice can be of primitive or non-primitive type.

Summary

Choice should only change at set and not in get. Current behavior does not adhere to this principle and needs to be changed, Moreover if a choice is overridden a warning message should notify the user that they are doing so.

Explanation with examples

Lets take a sample data model for the explanation

sample:
    type: object
    properties:
        name:
            type: string
        choice:   
            type: string
            enum:
                str_val:
                    type: string
                num_val:
                    type: int
                str_vals:
                    type: array of string
                config_obj:
                    type: object # lets say inside this object we have properties p1 and p2
        

Python code snippet:

s = api.Sample()
s.name = "new_sample"

# choice set by user
s.str_val = "str1"

# If now user tries to access another choice
# it we will return None
s.num_val

# we wont be restricting overriding of choices, we would just throw a warning
s.num_val = 23 # throws a warning "warning: old choice str_val overridden with new choice num_val"
s.str_vals = [70] # throws a warning "warning: old choice num_val overridden with new choice str_vals"

Note: We will follow the same principle in go.

Why are we not throwing Exception on getattr of other choice option

Now choice can be a primitive attribute or non-primitive attribute, if we throw an exception at getattr, then setting of the attributes inside choice object will be blocked, because for non-primitive types we generally have only getattr.
for example this snippet:

# user first sets choice and value for str_val
s.str_val = "str1"

# if we decide to throw an exception on getattr once a choice is set
s.num_val # will raise a exception 
s.num_val = 6 # but the above exception does not block us from setting the choice to num_val

# now consider non-primitive type config_obj
s.config_obj # will raise exception
# now if we want to set config_obj we first have need to access it and this will be blocked due to the exception
s.config_obj.p1 = "str1" # raises exception
s.config_obj.p2 = "str2" # raises exception

Why we are allowing to override choices

  1. If we are restrict overriding of choices then we must also expose a method which the user needs to preform before changing choice which becomes clunky and cumbersome for the user.
    for example:
# user first sets str_val
s.str_val = "str1"

# trying to set num val will throw exception
s.num_val = 13

# to avoid that
s.unset()
s.num_val = 13
s.unset()
s.str_vals = ["asd"]
s.unset()
s.config_obj.p1 = "str2" 

Also there are scenarios where user would want set different choices to the same config and fetch result and see results.

  1. Just exposing method like unset or reset might confuse the user , making users think that method unsets the whole sample object which include choice as well as the other attributes such as name in this case. Maybe we can have reset_choice or clear_choice and if choice needs to be there in the method names it defeats the whole purpose of implicitly unsetting the choice, the better option already exists which is the choice property which can be used by the user according to their need.

  2. It might break existing scripts if any, where overriding of choices are currently been done.

Scope

  1. Snappi
  2. GoSnappi

@ashutshkumr ashutshkumr added ux Impacts UX priority High Priority and removed py ux sdk gen labels Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority High Priority ux Impacts UX
Projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

5 participants