Use connection request v2 spec

[sajith]

Resolves #296. Work in progress, not quite there yet, etc. Updates in comments below.

[sajith]

The POST /connection controller must be able to receive the new connection requests in order to generate connection IDs. But validation of the request body happens before it hits the controller method, so in order to accept v2 format connection requests, the OpenAPI/Swagger definition must change. That might break a few things.

It is not quite clear to me if we should maintain backward compatibility with the original connection request format. If we want to maintain backward compatibility at least for a while, we will probably have to add a new endpoint to handle connection requests in v2 format?

[YufengXin]

@sajith adding a new /connection/v2 endpoint is probably a good idea, so no need to overhaul all the unittests for the time being? in the swagger.yaml in sdx_lc, I've added the new request connection scheme there.

[sajith]

@YufengXin A simpler solution might be to use oneOf, like so:

  /connection:
    post:
      operationId: place_connection
      requestBody:
        content:
          application/json:
            schema:
              oneOf:
                - $ref: '#/components/schemas/connection'
                - $ref: '#/components/schemas/connection_v2'
        required: true

Now I need to figure out how to write a connection_v2 schema. I tried converting Connection.json to YAML (using an online tool) and using that in the OpenAPI spec, but that does not pass connexion's validator.

[coveralls]

Pull Request Test Coverage Report for Build 9783166269

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 52.322%

---

Totals
Change from base Build 9586048784:	0.0%
Covered Lines:	758
Relevant Lines:	1473

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 9785415149

Details

• 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.2%) to 52.486%

---

Totals
Change from base Build 9586048784:	0.2%
Covered Lines:	763
Relevant Lines:	1478

---

💛 - Coveralls

[sajith]

@YufengXin @congwang09 I'm keeping this in draft state since this uses code that is not in the main branches of pce and datamodel, but please take a look when you get a chance.

The example connection request does not find a path. However we do generate a connection ID when it is not present (as in the case of v2 connection requests), and send a response like below, and a non-200 response code:

{
  "connection_id": "7e84c036-cc65-4257-9049-9b70992e8057",
  "reason": "Could not generate a traffic matrix",
  "status": "Failure"
}

[YufengXin]

The modification looks good to me -:)

For the unittest, in the draft PR in PCE,the test I added is this one:

https://github.com/atlanticwave-sdx/pce/blob/052788738691de677786b4c1bc496bafcb4a9043/tests/test_te_manager.py#L947

that uses this request connection in v2 format:

tests/data/test_request-amlight_zaoxi-p2p-v2.json

which finds the path on the three-topology

[coveralls]

Pull Request Test Coverage Report for Build 9788498633

Details

• 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.2%) to 52.486%

---

Totals
Change from base Build 9586048784:	0.2%
Covered Lines:	763
Relevant Lines:	1478

---

💛 - Coveralls

[sajith]

It would be useful to move tests/data/test_request-amlight_zaoxi-p2p-v2.json from pce to datamodel src/sdx_datamodel/data/requests/ so that we can use it here, eventually.

However, for the purpose of the issue (namely, generating a connection ID), I think the newly added test should be sufficient.

[YufengXin]

made a copy to the datamodel repo:

https://github.com/atlanticwave-sdx/datamodel/blob/130-conform-to-request-data-model-spec-20/src/sdx_datamodel/data/requests/test_request-amlight_zaoxi-p2p-v2.json

[coveralls]

Pull Request Test Coverage Report for Build 9811578134

Details

• 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.2%) to 52.486%

---

Totals
Change from base Build 9586048784:	0.2%
Covered Lines:	763
Relevant Lines:	1478

---

💛 - Coveralls

[sajith]

Ok, added a test that finds a solution.

Note that sdx-pce @ git+https://github.com/atlanticwave-sdx/pce@2a4dbb7 refers to a pce branch that I just pushed, just for testing. We'll need to bump up datamodel and then pce so that we can "properly" use the newest connection request file that was added to datamodel.

[YufengXin]

Cool! Now that the two tests worked, would you mind/have time to merge the PRs to main properly across the 4 repos with the right pointers in project files? -:)

Otherwise I'll do it early next Monday.

Cong would work on the test and I'll fix next week if these merges break anything

[sajith]

Well, as long as you don't mind dealing with the fallout... Let me see how much I can get done today. :-)

[coveralls]

Pull Request Test Coverage Report for Build 9813294445

Details

• 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.2%) to 52.486%

---

Totals
Change from base Build 9586048784:	0.2%
Covered Lines:	763
Relevant Lines:	1478

---

💛 - Coveralls

[sajith]

@YufengXin @congwang09 I went ahead and merged https://github.com/atlanticwave-sdx/datamodel/pull/131 and https://github.com/atlanticwave-sdx/pce/pull/195. Leaving https://github.com/atlanticwave-sdx/sdx-lc/pull/156 alone for now because I have some comments there.

Ok, now you get to deal with the fallout from the above merges, while I get to do the main thing I've got to do. For a while anyway. ;-)

[sajith]

This should probably wait for @congwang09. :-)