Skip to content

Servantify /connections endpoints#1770

Merged
jschaul merged 6 commits intodevelopfrom
servantify-connections
Sep 15, 2021
Merged

Servantify /connections endpoints#1770
jschaul merged 6 commits intodevelopfrom
servantify-connections

Conversation

@jschaul
Copy link
Member

@jschaul jschaul commented Sep 15, 2021

As part of https://wearezeta.atlassian.net/browse/SQCORE-693 before work on https://wearezeta.atlassian.net/browse/SQCORE-958 can begin.

Follow-up to #1726

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • changelog.d contains the following bits of information:
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

@jschaul jschaul force-pushed the servantify-connections branch from 074df29 to 1709867 Compare September 15, 2021 14:36
@jschaul jschaul changed the title [skip ci] Servantify connections Servantify /connections endpoints Sep 15, 2021
@jschaul jschaul marked this pull request as ready for review September 15, 2021 14:40
@jschaul jschaul force-pushed the servantify-connections branch from 1709867 to 3fce4f8 Compare September 15, 2021 14:56
Copy link
Member

@akshaymankar akshaymankar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix either the type of size or the Description of the listConnection endpoint.
Otherwise looks good.

Comment on lines 338 to 342
:> Description "You can have no more than 1000 connections in accepted or sent state"
:> ZUser
:> "connections"
:> QueryParam' '[Optional, Strict, Description "User ID to start from"] "start" UserId
:> QueryParam' '[Optional, Strict, Description "Number of results to return (default 100, max 500)"] "size" (Range 1 500 Int32)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The description says maximum is 1000, but size can only be upto 500. Which one is right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, no pagination, great!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can have no more than 1000 connections in general. The description is about connections in general, not about this request specifically. Arguably it belongs to the "create connections" endpoint, or to all endpoints, or to another documentation page. If it's confusing, I can remove it from this endpoint.. But otherwise, you can list up to 500 in this request, and paginate by using the start parameter. I'm not sure I follow the "no pagination, great" comment, can you explain?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the description from this endpoint as it's misleading perhaps and wasn't there before.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I follow the "no pagination, great" comment, can you explain?

I thought there was no pagination, but I see now there is start_id, which implies pagination.
But if we want to limit connections to 1000, why take size at all? Why not just return upto 1000 connections every time?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, this is a setting, so nvm about why not return all of them. Let's just implement pagination like we did for conversations.

@jschaul
Copy link
Member Author

jschaul commented Sep 15, 2021

unrelated flaky test failure on CI:

{"activation.code":"350069","activation.key":"2-FkP9syQwHq5K8wu1isFQyz1ARh__J4Ot1L4sMpsmM=","request":"N/A","msgs":["I","Activating"]}

{"user":"0841dcff-9f28-4197-86c6-21999e6c5f55","request":"N/A","msgs":["I","User activated"]}

{"user":"0841dcff-9f28-4197-86c6-21999e6c5f55","logger":"index.brig","msgs":["I","Indexing user"]}

{"request":"N/A","msgs":["I","getHandleInfo - local lookup"]}

{"user":"1ba28afb-f9c1-4909-bcb9-cfbd77e182d6","request":"N/A","msgs":["I","Creating user"]}

{"activation.key":"ke2mKi5AF77XGkzo2pMQOics0KWcrXi-KgCLgAsuiw0=","user":"1ba28afb-f9c1-4909-bcb9-cfbd77e182d6","request":"N/A","msgs":["I","Created email activation key/code pair"]}

{"activation.code":"723693","activation.key":"ke2mKi5AF77XGkzo2pMQOics0KWcrXi-KgCLgAsuiw0=","request":"N/A","msgs":["I","Activating"]}

{"user":"1ba28afb-f9c1-4909-bcb9-cfbd77e182d6","request":"N/A","msgs":["I","User activated"]}

{"user":"1ba28afb-f9c1-4909-bcb9-cfbd77e182d6","logger":"index.brig","msgs":["I","Indexing user"]}

{"user":"1d7354b6-1687-4404-a882-2784cd4b7b8d","request":"N/A","msgs":["I","Creating user"]}

{"activation.key":"_j3PPKggXgm-8UXMeoa2KS6YkBKFFtlAc-oOC8uErc0=","user":"1d7354b6-1687-4404-a882-2784cd4b7b8d","request":"N/A","msgs":["I","Created email activation key/code pair"]}

{"activation.code":"949000","activation.key":"_j3PPKggXgm-8UXMeoa2KS6YkBKFFtlAc-oOC8uErc0=","request":"N/A","msgs":["I","Activating"]}

{"user":"1d7354b6-1687-4404-a882-2784cd4b7b8d","request":"N/A","msgs":["I","User activated"]}

{"user":"1d7354b6-1687-4404-a882-2784cd4b7b8d","logger":"index.brig","msgs":["I","Indexing user"]}

OK (0.63s)

    order-name (prefix match)

      new-index:                                                                {"logger":"cassandra.brig","msgs":["I","Known hosts: [datacenter1:rack1:10.233.113.204:9042]"]}

{"logger":"cassandra.brig","msgs":["I","New control connection: datacenter1:rack1:10.233.113.204:9042#<socket: 112>"]}

{"user":"db7175c4-715c-4977-bec7-f0c6673a9d9e","request":"N/A","msgs":["I","Creating user"]}

{"activation.key":"Odm5yljr-MnrICl9dsxIWlm8t3Y3v3EmUouEq9FE4Cw=","user":"db7175c4-715c-4977-bec7-f0c6673a9d9e","request":"N/A","msgs":["I","Created email activation key/code pair"]}

{"activation.code":"250948","activation.key":"Odm5yljr-MnrICl9dsxIWlm8t3Y3v3EmUouEq9FE4Cw=","request":"N/A","msgs":["I","Activating"]}

{"user":"db7175c4-715c-4977-bec7-f0c6673a9d9e","request":"N/A","msgs":["I","User activated"]}

{"user":"db7175c4-715c-4977-bec7-f0c6673a9d9e","logger":"index.brig","msgs":["I","Indexing user"]}

{"user":"241babff-208e-4a42-89b1-1f907afdc9e3","request":"N/A","msgs":["I","Creating user"]}

{"activation.key":"AFrWoGll5v8cYC32lQg8huP2Kaj0YqPfEFlcfp4exgY=","user":"241babff-208e-4a42-89b1-1f907afdc9e3","request":"N/A","msgs":["I","Created email activation key/code pair"]}

{"activation.code":"519460","activation.key":"AFrWoGll5v8cYC32lQg8huP2Kaj0YqPfEFlcfp4exgY=","request":"N/A","msgs":["I","Activating"]}

{"user":"241babff-208e-4a42-89b1-1f907afdc9e3","request":"N/A","msgs":["I","User activated"]}

{"user":"241babff-208e-4a42-89b1-1f907afdc9e3","logger":"index.brig","msgs":["I","Indexing user"]}

FAIL (0.53s)

        test/integration/API/Search.hs:271:

        Expected order: name match, name prefix match.

        

        Since this test fails sporadically for unknown reasons here is some debug info:

        results: [Contact {contactQualifiedId = Qualified {qUnqualified = ae05eb04-4569-4b7a-a4ef-bf2ad7edc998, qDomain = Domain {_domainText = "federation-test-helper.test-nqkj8m5n852s.svc.cluster.local"}}, contactName = "\38665\13942\48526\37810\42521\46653\63983\40328\15514\22443\50688\7300\5857\50152\10959\64868\17745\34113\40985\14035\4168\544\64871\8084\54964\36673\23796\8697\16669\6328\1106\30649\20491\13292\34869\502\38018\26735\32726\336\51207\23539\35365\22485\26941\13897\40064\22508\29351\45257\45567\22289\51923\46453\54124\48610\19645\11191\47559\27857\4161\51709\13431\15434\43656\22054\48932\15553\13652suffix", contactColorId = Just 0, contactHandle = Nothing, contactTeam = Nothing},Contact {contactQualifiedId = Qualified {qUnqualified = 8c1052dd-9abb-4fd0-b0c8-056cfb7167ee, qDomain = Domain {_domainText = "federation-test-helper.test-nqkj8m5n852s.svc.cluster.local"}}, contactName = "\38665\13942\48526\37810\42521\46653\63983\40328\15514\22443\50688\7300\5857\50152\10959\64868\17745\34113\40985\14035\4168\544\64871\8084\54964\36673\23796\8697\16669\6328\1106\30649\20491\13292\34869\502\38018\26735\32726\336\51207\23539\35365\22485\26941\13897\40064\22508\29351\45257\45567\22289\51923\46453\54124\48610\19645\11191\47559\27857\4161\51709\13431\15434\43656\22054\48932\15553\13652", contactColorId = Just 0, contactHandle = Nothing, contactTeam = Nothing}]

        searchedWord: 霉㙶붎鎲ꘙ똽璘鶈㲚垫였ᲄᛡ쏨⫏ﵤ䕑蕁ꀙ㛓၈Ƞﵧᾔ횴轁峴⇹䄝ᢸђ瞹個㏬蠵Ƕ钂桯翖Ő젇寳訥埕椽㙉鲀埬犧냉뇿圑쫓땵퍬뷢䲽⮷맇泑၁짽㑷㱊ꪈ嘦뼤㳁㕔

        expected: [Qualified {qUnqualified = 8c1052dd-9abb-4fd0-b0c8-056cfb7167ee, qDomain = Domain {_domainText = "federation-test-helper.test-nqkj8m5n852s.svc.cluster.local"}},Qualified {qUnqualified = ae05eb04-4569-4b7a-a4ef-bf2ad7edc998, qDomain = Domain {_domainText = "federation-test-helper.test-nqkj8m5n852s.svc.cluster.local"}}]

         but got: [Qualified {qUnqualified = ae05eb04-4569-4b7a-a4ef-bf2ad7edc998, qDomain = Domain {_domainText = "federation-test-helper.test-nqkj8m5n852s.svc.cluster.local"}},Qualified {qUnqualified = 8c1052dd-9abb-4fd0-b0c8-056cfb7167ee, qDomain = Domain {_domainText = "federation-test-helper.test-nqkj8m5n852s.svc.cluster.local"}}]

      old-index:                                                                {"logger":"cassandra.brig","msgs":["I","Known hosts: [datacenter1:rack1:10.233.113.204:9042]"]}

{"logger":"cassandra.brig","msgs":["I","New control connection: datacenter1:rack1:10.233.113.204:9042#<socket: 84>"]}

{"user":"e0afd20d-04ce-461b-8ee4-1a6d96ca2992","request":"N/A","msgs":["I","Creating user"]}

{"activation.key":"Iw7k8CPyRdg4IXMvFQIKTQXldlCeCaJYfPO_gEbALTo=","user":"e0afd20d-04ce-461b-8ee4-1a6d96ca2992","request":"N/A","msgs":["I","Created email activation key/code pair"]}

@jschaul jschaul merged commit c829a7d into develop Sep 15, 2021
@jschaul jschaul deleted the servantify-connections branch September 15, 2021 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants