xenanetworks / open-automation-core

🚀 XOA framework for test execution, tester management, and data subscription.
https://docs.xenanetworks.com/projects/xoa-core
Apache License 2.0
2 stars 0 forks source link

The port_identities field's schema should be changed #16

Closed leonardhyu closed 1 year ago

leonardhyu commented 1 year ago

It's hard to make a schema of port_identities, because property names are not deterministic. It looks like an array but not really array. Suggestion is to make it into:

"port_identities": [
        {
            "tester_id": "598e592ceb28ce73b95d270a1b028ff3",
            "tester_index": 0,
            "module_index": 1,
            "port_index": 0
        },
        {
            "tester_id": "598e592ceb28ce73b95d270a1b028ff3",
            "tester_index": 0,
            "module_index": 1,
            "port_index": 1
        }
    ],

Please think about the json schema when designing data structure in the future.

This is VERY IMPORTANT because it dictates our XOA API. We shouldn't change our API after release unless it is really really necessary. Changing interfaces is always a pain to any developer, including ourselves.

leonardhyu commented 1 year ago

This is what the schema could look like (writeen in yaml). You are more than welcome to comment on it.

HelloWorldTestConfig:
      description: Configuration of the "Hello World" ANLT test
      type: object
      properties:
        username:
          description: Username
          type: string
          example: 'OpenAutomation'
        test_name:
          description: Name of the test that this configuration is for
          type: string
          example: 'ANLT'
        destination:
          description: The URL of the endpoint for the Worker to send statistics, alerts, notifications, etc.
          type: string
          format: url
          example: 'https://fb3c8b2e-b22f-47aa-8122-15d635ab6d0d.mock.pstmn.io/temp'
        port_identities:
          description: List of ports that this test will reserve and use
          type: array
          items:
            $ref: '#/components/schemas/PortIdentityObject'
        config:
          type: object
    PortIdentityObject:
      type: object
      properties:
        tester_id:
          $ref: '#/components/schemas/TesterId'
        tester_index:
          type: integer
        module:index:
          type: integer
        port_index:
          type: integer
leonardhyu commented 1 year ago

Please consider fixing this in XOA Core on Monday. It is currently blocking us from shipping our XOA API

ArtemConstantinov commented 1 year ago

We can not add tester index inside of the dataset, coz dataset is can be stored in precision storage and the tester index is an dynamic value, and that value will be changed for all testers if an tester which was added first in list will be removed then second tester automatically will take index 0. I apply those changes to the code, tomorrow will work on Return Tester ID when add new tester

ArtemConstantinov commented 1 year ago

@RonDingDing @fpfeng Please modify the test suites and config converters to the representation from the first Leo's comment, I apply this changes to the XOA-Core new release.

For @Tracysq is zero changes coz her dataset is different then representation in this issue

leonardhyu commented 1 year ago

We can not add tester index inside of the dataset, coz dataset is can be stored in precision storage and the tester index is an dynamic value, and that value will be changed for all testers if an tester which was added first in list will be removed then second tester automatically will take index 0. I apply those changes to the code, tomorrow will work on Return Tester ID when add new tester

The dataset having tester index is not something I made. I got that dataset from XOA API, so it must been already there in the code. @ArtemConstantinov

leonardhyu commented 1 year ago

@ArtemConstantinov If you want to remove the tester_index, please discuss with Ron and Frank. What I can see is that if tester_index is removed, there must be another way to determine the index of the tester.

leonardhyu commented 1 year ago

@ArtemConstantinov To change the test config, please remember to change:

  1. xoa-converter
  2. xoa-core
ArtemConstantinov commented 1 year ago

Index, and dataset is updated on the core, will push the changes a bit later. There is no requirement to change converter code from mine side. Needs only to change the code from side of testsuites

ArtemConstantinov commented 1 year ago

Done, xoa-core side

fpfeng commented 1 year ago

@ArtemConstantinov Error popup after upgrade xoa-core 1.0.6, seems like when it enter self.__observer.emit(const.CONNECTED, self.info()) , self.dataset having none value index .

Traceback (most recent call last):
  File "/home/chf/xena/open-automation-test-suites/2889_main.py", line 26, in test_playground
    await c.add_tester(t)
  File "/home/chf/xena/open-automation-test-suites/__pypackages__/3.11/lib/xoa_core/controller.py", line 106, in add_tester
    return await self.__resources.add_tester(credentials)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/chf/xena/open-automation-test-suites/__pypackages__/3.11/lib/xoa_core/core/resources/controller.py", line 45, in add_tester
    await new_resource.connect()  # TesterCommunicationError
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/chf/xena/open-automation-test-suites/__pypackages__/3.11/lib/xoa_core/core/resources/resource/facade.py", line 105, in connect
    self.__observer.emit(const.CONNECTED, self.info())
                                          ^^^^^^^^^^^
  File "/home/chf/xena/open-automation-test-suites/__pypackages__/3.11/lib/xoa_core/core/resources/resource/facade.py", line 149, in info
    return TesterInfoModel.parse_obj(asdict(self.dataset))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "pydantic/main.py", line 526, in pydantic.main.BaseModel.parse_obj
  File "pydantic/main.py", line 342, in pydantic.main.BaseModel.__init__
pydantic.error_wrappers.ValidationError: 1 validation error for TesterInfoModel
index
  none is not an allowed value (type=type_error.none.not_allowed)
ArtemConstantinov commented 1 year ago

Firstly, remove your storage file, after you need to adjust test-suite and converter to use list in port_identities instead of a dict. Also changed a name of the method get_test_list in to get_testers_info

fpfeng commented 1 year ago

test suite data model updated and store file deleted, the above error popup when calling add_tester before load test config.

    new = (
        types.Credentials(product=types.EProductType.VALKYRIE, host="192.168.1.198"),
        # types.Credentials(product=types.EProductType.VALKYRIE, host="demo.xenanetworks.com"),
        # types.Credentials(product=types.EProductType.VALKYRIE, host="87.61.110.118"),
    )
    c = await controller.MainController()
    c.register_lib("./plugin2889")
    for t in new:
        await c.add_tester(t)
fpfeng commented 1 year ago

@ArtemConstantinov Sorry my fault, now I am sure the error(none index) exists, please check. If I temporary change the index as fixed 0 cause I only have one single tester, another error popup.

Traceback (most recent call last):
  File "/home/chf/xena/open-automation-test-suites/2889_main.py", line 32, in test_playground
    info = c.get_test_suite_info("RFC-2889")
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/chf/xena/open-automation-test-suites/__pypackages__/3.11/lib/xoa_core/controller.py", line 80, in get_test_suite_info
    return self.suites_library.suite_info(name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/chf/xena/open-automation-test-suites/__pypackages__/3.11/lib/xoa_core/core/test_suites/controller.py", line 42, in suite_info
    schema=suite.model_class.schema_json(),
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "pydantic/main.py", line 675, in pydantic.main.BaseModel.schema_json
  File "pydantic/main.py", line 664, in pydantic.main.BaseModel.schema
  File "pydantic/schema.py", line 186, in pydantic.schema.model_schema
  File "pydantic/schema.py", line 580, in pydantic.schema.model_process_schema
  File "pydantic/schema.py", line 621, in pydantic.schema.model_type_schema
  File "pydantic/schema.py", line 254, in pydantic.schema.field_schema
  File "pydantic/schema.py", line 526, in pydantic.schema.field_type_schema
  File "pydantic/schema.py", line 921, in pydantic.schema.field_singleton_schema
  File "<frozen abc>", line 123, in __subclasscheck__
TypeError: issubclass() arg 1 must be a class
leonardhyu commented 1 year ago

Maybe we should remove that tester_index completely

leonardhyu commented 1 year ago

In either xoa's json or xoa rest api, a port is already identified by tester_id + module_index + port_index. Using this way, we can always find the port no matter what the test resource inventory looks like. But adding tester_index will break this freedom in that it forces the tester to be on that position, and if the inventory is changed, e.g. the tester is moved to another position, then the config won't work at all.

So we should remove the tester_index from the json and the rest api payload, and let the UI to render the tester's index. Inside the test suite, there will be no tester index but only the tester id because it doesn't care about the tester's position in the inventory.

fpfeng commented 1 year ago

@RonDingDing just talk to me that your guys design port_identities as dictionary long time ago because its keys could mapping to "port_slot" value for fast switch slot, he needs to redesign if your guys still need this feature.

fpfeng commented 1 year ago

https://github.com/xenanetworks/open-automation-core/blob/a8664d0edfd276f020ecad3aa24bf88fd5d98270/xoa_core/core/test_suites/datasets.py#L22-L30

@ArtemConstantinov @leonardhyu Could we change name as f"P-{self.tester_id}-{self.module_index}-{self.port_index}", add tester_id is to make sure it is unique name. BTW tester_index is not yet removed.