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

fix: Add pagination to AzureDevops modified files call #5298

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

bub3n
Copy link

@bub3n bub3n commented Feb 3, 2025

what

why

  • At our company we use Atlantis for terraform automatization, we bumped into the problem if we have lots of changes across multiple projects that Atlantis is not planning all of them

tests

  • I have tested it in our stack
  • also I fixed test and I was able to run make test without any errors

references

@bub3n bub3n requested review from a team as code owners February 3, 2025 14:14
@bub3n bub3n requested review from jamengual, lukemassa and nitrocode and removed request for a team February 3, 2025 14:14
@dosubot dosubot bot added go Pull requests that update Go code provider/azuredevops labels Feb 3, 2025
@github-actions github-actions bot added docs Documentation dependencies PRs that update a dependency file labels Feb 3, 2025
@jamengual
Copy link
Contributor

@bub3n Thanks for the PR. Could you write some tests for this?

Thanks.

@jamengual jamengual added the waiting-on-response Waiting for a response from the user label Feb 4, 2025
@bub3n
Copy link
Author

bub3n commented Feb 4, 2025

@jamengual sure, I'll prepare something

@bub3n
Copy link
Author

bub3n commented Feb 4, 2025

@jamengual Tried to add test for new client with top and skip arguments.

Idea behind this is to test client without and with the behavior so we know if it won't break the default setup

@X-Guardian
Copy link
Contributor

Hi @bub3n, I started to have a look at this PR, but it looks like you have got a lot of unrelated changes committed. Can you review these and remove anything unrelated to the description in the PR. Thanks.

@bub3n
Copy link
Author

bub3n commented Feb 4, 2025

Yeah, my bad. Working on a fix. Sorry for troubles

@X-Guardian
Copy link
Contributor

No problem. I assume this fixes the pagination of the relevant Azure DevOps API call, but I don't think we need additional server parameters to adjust the pagination. We are trying to avoid adding server parameters that aren't necessary.

@bub3n
Copy link
Author

bub3n commented Feb 4, 2025

@X-Guardian unfortunately it does not. I guess you are right, my bad. I'll rework it for pagination.

BTW what number of elements would you say is good per "page"? 100 which is their API default?

@X-Guardian
Copy link
Contributor

Yes, I'm sure the default will be fine.

@bub3n bub3n reopened this Feb 10, 2025
@bub3n
Copy link
Author

bub3n commented Feb 10, 2025

Hello again, sorry for previous problems caused.

This time I tried to prepare it so it uses the pagination. I also fixed the test. And I ran make test which was fine. Also I tested it on our infra if everything seems okay. And Atlantis plan went fine and with more changes (as expected for the commit).

@X-Guardian
Copy link
Contributor

No problem @bub3n. Good to see Azure DevOps getting some love.

Some linting and formatting fixes needed.

Can you also update the PR title to more accurately describe the change? (This is used for the changelog entry)

@bub3n
Copy link
Author

bub3n commented Feb 10, 2025

Sure, I'll fix it. Thx for the feedback.

@bub3n bub3n changed the title fix: Parametrize number of files in diff from azure devops endpoint fix: Add pagination to AzureDevops modified files call Feb 10, 2025
@lukemassa
Copy link
Contributor

I recently merged #5309 which caused a conflict here. I'm happy to fix it, or you can if you want (it's just the err check has to move down a bit to right after the call to GetDiffs)

@bub3n
Copy link
Author

bub3n commented Feb 11, 2025

@lukemassa please fix it if you don't mind. I'll try to fix the linting problems.

@X-Guardian X-Guardian removed build Relating to how we build Atlantis dependencies PRs that update a dependency file provider/gitlab github-actions labels Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation go Pull requests that update Go code provider/azuredevops waiting-on-response Waiting for a response from the user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Atlantis plans less projects than anticipated by modified files
4 participants