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

Deflake invitation too many pending #3092

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

jschaul
Copy link
Member

@jschaul jschaul commented Feb 15, 2023

deflake post /teams/:tid/invitations - 403 too many pending'

    FAIL
            Exception: Assertions failed:
             1: 403 =/= 201
             2: Just too-many-team-invitations =/= Nothing

@jschaul jschaul requested a review from mdimjasevic February 15, 2023 18:46
@jschaul jschaul force-pushed the deflake-invitation-too-many-pending branch from 939953d to ac9d2d4 Compare February 15, 2023 18:47
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Feb 15, 2023
FAIL
        Exception: Assertions failed:
         1: 403 =/= 201
         2: Just too-many-team-invitations =/= Nothing
@jschaul jschaul force-pushed the deflake-invitation-too-many-pending branch from ac9d2d4 to 908490e Compare February 15, 2023 18:51
-- therefore we increase the timeout from default 10 to 300 seconds
let longerTimeout = opts {Opt.optSettings = (Opt.optSettings opts) {Opt.setTeamInvitationTimeout = 300}}
withSettingsOverrides longerTimeout $ do
forM_ emails $ postInvitation brig tid inviter . stdInvitationRequest
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know what's the setMaxTeamSize value in options passed to the test, but if it's big, could it be that doing this sequentially takes a bit too long? I was planning to make a very little team size limit, along these lines:

  let smallTeam =
        opts
          { Opt.optSettings =
              (Opt.optSettings opts)
                { Opt.setTeamInvitationTimeout = 300,
                  Opt.setMaxTeamSize = 5
                }
          }
  withSettingsOverrides smallTeam $ do

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 was thinking since tests already run in parallel, we may not gain that much by spawning even more threads, as these are just simple http requests that should be at milisecond response time level in theory (when there isn't anything else ongoing and enough CPU/memory available). Right now the team size is 32 that is inside the hack/helm_vars/... override. Setting maxTeamSize to something smaller is a good idea though, your suggestion makes sense, feel free to adjust this test to this in a separate PR (I see you already merged this one as-is).

@mdimjasevic mdimjasevic merged commit 4bec127 into develop Feb 16, 2023
@mdimjasevic mdimjasevic deleted the deflake-invitation-too-many-pending branch February 16, 2023 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants