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

gitlab throttling + adaptation to gitlab api changes + more logging for troubleshooting #256

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

yaman
Copy link

@yaman yaman commented Jan 11, 2023

  • gitlab api was rejecting requests for large groups with many projects, added throttling
  • gitlab api changes broke the whole requests, changed the request types and structure to allow group/subgroup projects to be retrieved successfully
  • added configuration for debug option to enable/disable verbose logging
  • added configuration to define main group to only allow projects to be monitored under main group recursively

Copy link
Owner

@marcells marcells left a comment

Choose a reason for hiding this comment

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

Hi, thanks for your PR. I've left some comments in it.

@@ -111,6 +111,10 @@ monitor.run();

// helpers
function tryExpandEnvironmentVariables(monitorConfiguration, serviceConfiguration) {
if (process.env.TOKEN) {
Copy link
Owner

Choose a reason for hiding this comment

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

Please revert this change. This should happen automatically by tryExpandEnvironmentVariable, if expandEnvironmentVariables is set to true.

@@ -1,11 +1,16 @@
version: '3'
version: '3.9'
Copy link
Owner

Choose a reason for hiding this comment

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

Please revert all changes in this file.

@@ -20,4 +20,4 @@
}
Copy link
Owner

Choose a reason for hiding this comment

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

Please revert all changes in this file.

| `intervals` | How often (in integer of milliseconds) ...
| `additional_query` | Add [additional query parameters](https://gitlab.com/help/api/projects.md) so not too many projects are fetched.
| `main_group` | Gitlab needs a main group to analyze the project only you want to monitor, otherwise, you will get the whole world.
Copy link
Owner

Choose a reason for hiding this comment

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

What happens, if this property is not set? Will existing GitLab configurations crash after an upgrade of the build monitor?

@@ -14,4 +14,4 @@ ONBUILD ADD config.json /build-mon/app/config.json

Copy link
Owner

Choose a reason for hiding this comment

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

Please revert all changes in this file.

@@ -1,100 +1,44 @@
name: Build and Release
# This workflow uses actions that are not certified by GitHub.
Copy link
Owner

Choose a reason for hiding this comment

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

Please revert all changes in this file.

pipelines = pipelines.filter(function(pipeline) {
//console.log(pipeline);
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove unnecessary code.

if (getHasWarnings(detailed_status)) {
return '#ffa500';
}
// if (getHasWarnings(detailed_status)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this removed? How are warnings handled now?

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