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

[server] bump GitLab API library #13001

Merged
merged 1 commit into from
Sep 19, 2022
Merged

[server] bump GitLab API library #13001

merged 1 commit into from
Sep 19, 2022

Conversation

AlexTugarev
Copy link
Member

@AlexTugarev AlexTugarev commented Sep 15, 2022

Description

PR updates GitLab API library, adds tests to cover changes and verify that the paginated requests now works as expected.

Related Issue(s)

Fixes #12970

Screen Shot 2022-09-16 at 13 44 56

How to test

  • checked context parser tests for GitLab are still ✅
    • export GITPOD_TEST_TOKEN_GITLAB='{ "value": "F00....", "scopes":[] }'
    • cd components/server && npx mocha --opts mocha.opts './**/gitlab-*.spec.ts' --exclude './node_modules/**'
  • add manual test to cover the Projects.all request
    • run createManyRepos to create
  • verify that more than 100 projects/repos are found on /new
  1. verify the check points in comments bellow

Release Notes

Update GitLab API library, which fixes paginated API requests.

Documentation

Werft options:

  • /werft with-preview

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-at-gl.1 because the annotations in the pull request description changed
(with .werft/ from main)

@AlexTugarev AlexTugarev changed the title wip [server] bump GitLab API library Sep 15, 2022
const projectsWithAccess = await api.Projects.all({
min_access_level: "40",
perPage: 100,
pagination: "keyset",
Copy link
Member Author

@AlexTugarev AlexTugarev Sep 15, 2022

Choose a reason for hiding this comment

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

Hi @gtsiolis! Would you like to test this change on the preview environment? Maybe you know how to find the 101 repo in the GitLab org/group?
It's not an urgent request, actually just trying to test-ride the new release of the API library.

Copy link
Contributor

Choose a reason for hiding this comment

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

@AlexTugarev I don't have maintainer access to such a long list of repositories to try this but took 10 minutes to create a list of 101 repositories via command line in a reproduction group.

However, I can already find the 101th repository in Gitpod.io, see screenshot below. 💭

For this preview environment, when I add the GitLab integration I'm stuck in the fetching repositories screen with the following error in console.

Error: Request getProviderRepositoriesForUser failed with message: Response code 405 (Method Not Allowed)
Gitpod.io This preview environment
Screenshot 2022-09-16 at 2 27 13 PM Screenshot 2022-09-16 at 2 24 35 PM

Copy link
Member Author

Choose a reason for hiding this comment

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

@gtsiolis, in the meantime, I was able to create a test script to create ~200 test projects in https://gitlab.com/alex-gp-test-group?page=1 and wrote a unit test for the request which was expected to be broken. not to forget, those tests are not automated, as they require a personal OAuth token in the environ.

That said, I'd be more than happy if you could retest after next build is deployed to preview. 🙏🏻

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'll make a screenshot to the description as well, once I could verify it myself.

Copy link
Contributor

Choose a reason for hiding this comment

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

... in the meantime, I was able to create a test script to create ~200 test projects ...

🙂

... wrote a unit test for the request which was expected to be broken. not to forget, those tests are not automated, as they require a personal OAuth token ...

It could be nice if we could use a dedicated group for such integrations tests, right? Cross-posting a relevant comment from #5362 (comment) for visibility.

... if you could retest after next build is deployed to preview

I'll keep an eye on the build[1].

I'll make a screenshot to the description as well

Thank you! 🙇

Copy link
Contributor

Choose a reason for hiding this comment

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

@AlexTugarev I tested this again and works as expected. I can see the 101th repository. ✔️

Copy link
Member Author

Choose a reason for hiding this comment

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

@gtsiolis, see the updated description. Also, sorry to have you seen the broken state in between. 🤦🏻‍♂️

@AlexTugarev AlexTugarev force-pushed the at/gl branch 2 times, most recently from c09ec17 to 981c285 Compare September 16, 2022 11:30
@roboquat roboquat added size/XL and removed size/L labels Sep 16, 2022
verified: true,
},
});
expect(result.length).to.be.greaterThan(200);
Copy link
Member Author

Choose a reason for hiding this comment

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

better this not-automated test, than nothing.

Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

UX LGTM. Approving this but this will need an approval also from @gitpod-io/engineering-webapp.

Copy link
Member

@svenefftinge svenefftinge left a comment

Choose a reason for hiding this comment

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

I just read through the code, but haven't tested. Adding hold in case this needs more manual testing.

/hold

components/server/src/gitlab/api.ts Outdated Show resolved Hide resolved
components/server/src/gitlab/api.ts Outdated Show resolved Hide resolved
this also adds test for changed API and a test to verify that `Projects.all` is paginated as expected.
@metcalfc
Copy link
Contributor

We have folks blocked on this for their evaluations. Please merge as soon as possible.

@AlexTugarev
Copy link
Member Author

/werft run

Testing the GitLab features to get a sense of current state of GitLab features affected by this PR. 'Merging then, if nothing seems to be broken.

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-at-gl.8 because the annotations in the pull request description changed
(with .werft/ from main)

@AlexTugarev
Copy link
Member Author

/hold cancel

@roboquat roboquat merged commit cc10536 into main Sep 19, 2022
@roboquat roboquat deleted the at/gl branch September 19, 2022 07:32
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/XL team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Impossible to add some GitLab repositories as a Project when you're a maintainer of more than 100 repositories
5 participants