Crontab-Docker #37
Open
Crontab-Docker #37
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adambean
previously approved these changes
May 1, 2023
Adambean
dismissed
their stale review
This shouldn't have approved the whole thing. Just one file.
Adambean
requested changes
May 1, 2023
| @@ -75,8 +75,7 @@ public function configure(): void | |||
| ->setDescription("Sync LDAP users and groups with a Gitlab CE/EE self-hosted installation.") | |||
| ->addOption("dryrun", "d", InputOption::VALUE_NONE, "Dry run: Do not persist any changes.") | |||
| ->addOption("continueOnFail", null, InputOption::VALUE_NONE, "Do not abort on certain errors. (Continue running if possible.)") | |||
| ->addArgument("instance", InputArgument::OPTIONAL, "Sync with a specific instance, or leave unspecified to work with all.") | |||
| ; | |||
| ->addArgument("instance", InputArgument::OPTIONAL, "Sync with a specific instance, or leave unspecified to work with all."); | |||
There was a problem hiding this comment.
That shouldn't have been slipped in.
| @@ -1103,8 +1101,7 @@ private function deployGitlabUsersAndGroups(array $config, string $gitlabInstanc | |||
|
|
|||
| $this->logger->debug("Gitlab: Connecting"); | |||
| $gitlab = \Gitlab\Client::create($gitlabConfig["url"]) | |||
| ->authenticate($gitlabConfig["token"], \Gitlab\Client::AUTH_HTTP_TOKEN) | |||
| ; | |||
| ->authenticate($gitlabConfig["token"], \Gitlab\Client::AUTH_HTTP_TOKEN); | |||
There was a problem hiding this comment.
That shouldn't have been slipped in.
| @@ -308,7 +307,6 @@ private function validateConfig(array &$config, array &$problems = null): bool | |||
|
|
|||
| $this->logger->$type(sprintf("Configuration: %s", $message)); | |||
| $problems[$type][] = $message; | |||
|
|
|||
There was a problem hiding this comment.
Anonymous function: The empty line here should have been kept.
Comment on lines
-1660
to
+1661
|
|
||
| !$this->dryRun ? ($gitlabGroupMember = $gitlab->api("groups")->addMember($gitlabGroupId, $gitlabUserId, $config["gitlab"]["options"]["newMemberAccessLevel"])) : $this->logger->warning("Operation skipped due to dry run."); | ||
|
|
||
| try { | ||
| !$this->dryRun ? ($gitlabGroupMember = $gitlab->api("groups")->addMember($gitlabGroupId, $gitlabUserId, $config["gitlab"]["options"]["newMemberAccessLevel"])) : $this->logger->warning("Operation skipped due to dry run."); | ||
| } catch (\Exception $e) { | ||
| $this->logger->error(sprintf("Gitlab failure: %s", $e->getMessage()), ["error" => $e]); | ||
| } |
There was a problem hiding this comment.
Perhaps somewhat dangerous, would be better as a "--continue-on-errors" option.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I rebuilt the Dockerfile, and the new Docker can run independently and periodically execute the gitlab-ldap-sync task. In addition, I added a try...catch... block to line 1657 of LdapSyncCommand.php, so that even if there is an error, the program will continue to synchronize the remaining users.