From 77750abca7de2bab253c0f5208a75352ba7551bb Mon Sep 17 00:00:00 2001 From: Morg42 <43153739+Morg42@users.noreply.github.com> Date: Thu, 14 Nov 2024 09:54:53 +0100 Subject: [PATCH] githubplugin: remove init_repos, update error handling, improve rate limit display --- githubplugin/__init__.py | 145 ++++++++++-------------- githubplugin/webif/__init__.py | 43 +++---- githubplugin/webif/templates/index.html | 133 +++++++++------------- 3 files changed, 126 insertions(+), 195 deletions(-) diff --git a/githubplugin/__init__.py b/githubplugin/__init__.py index ae3f70f94..a5f61aefa 100644 --- a/githubplugin/__init__.py +++ b/githubplugin/__init__.py @@ -49,7 +49,8 @@ def loggerr(self, msg): self.logger.error(msg) raise GPError(msg) - def __init__(self, repo='plugins', apikey='', auth=None, logger=None, **kwargs): + def __init__(self, dt, repo='plugins', apikey='', auth=None, logger=None, **kwargs): + self.dt = dt self.logger = logger self.apikey = apikey # allow auth only, if apikey is set @@ -119,7 +120,7 @@ def get_rate_limit(self): rl = self._github.get_rate_limit() allow = rl.core.limit remain = rl.core.remaining - backoff = (rl.core.reset.replace(tzinfo=None) - dt.datetime.now()).total_seconds() + backoff = (rl.core.reset - self.dt.now()).total_seconds() if backoff < 0: backoff = 0 except Exception as e: @@ -159,15 +160,19 @@ def get_pulls(self, fetch=False) -> bool: self.logger.debug('fetching pulls from github') self.pulls = {} - for pull in self.git_repo.get_pulls(): - self.pulls[pull.number] = { - 'title': pull.title, - 'pull': pull, - 'git_repo': pull.head.repo, - 'owner': pull.head.repo.owner.login, - 'repo': pull.head.repo.name, - 'branch': pull.head.ref - } + try: + for pull in self.git_repo.get_pulls(): + self.pulls[pull.number] = { + 'title': pull.title, + 'pull': pull, + 'git_repo': pull.head.repo, + 'owner': pull.head.repo.owner.login, + 'repo': pull.head.repo.name, + 'branch': pull.head.ref + } + except AttributeError: + self.logger.warning('github object not created. Check rate limit.') + return False return True @@ -184,8 +189,12 @@ def get_forks(self, fetch=False) -> bool: return True self.forks = {} - for fork in self.git_repo.get_forks(): - self.forks[fork.full_name.split('/')[0]] = {'repo': fork} + try: + for fork in self.git_repo.get_forks(): + self.forks[fork.full_name.split('/')[0]] = {'repo': fork} + except AttributeError: + self.logger.warning('github object not created. Check rate limit.') + return False return True @@ -294,7 +303,6 @@ def __init__(self, sh): # 'rel_wt_path': os.path.join('..', '..', wt_path), # relativer Pfad zum Worktree vom Repo aus # 'link': os.path.join('plugins', f'priv_{plugin}'), # relativer Pfad-/Dateiname des Plugin-Symlinks unterhalb von shng # 'rel_link_path': os.path.join(wt_path, plugin), # relativer Pfad des Pluginordners "unterhalb" von plugins/ - # 'force': False, # vorhandene Dateien überschreiben # 'repo': repo, # git.Repo(path) # 'clean': bool # repo is clean and synced? # }, @@ -306,7 +314,7 @@ def __init__(self, sh): self.repo_path = os.path.join('plugins', 'priv_repos') self.gh_apikey = self.get_parameter_value('app_token') - self.gh = GitHubHelper(apikey=self.gh_apikey, logger=self.logger) + self.gh = GitHubHelper(self._sh.shtime, apikey=self.gh_apikey, logger=self.logger) self.init_webinterface(WebInterface) @@ -314,7 +322,7 @@ def __init__(self, sh): # methods for handling local repos # - def read_repos_from_dir(self): + def read_repos_from_dir(self, exc=False): # clear stored repos self.repos = {} @@ -374,43 +382,43 @@ def read_repos_from_dir(self): 'rel_wt_path': os.path.join('..', '..', wt_path), 'link': str(item), 'rel_link_path': str(target), - 'force': False, 'repo': repo, } - self.repos[id]['clean'] = self.is_repo_clean(id) + self.repos[id]['clean'] = self.is_repo_clean(id, exc) - def init_repo(self, name, owner, plugin, branch=None, force=False) -> bool: - - # check if name exists in repos or link exists + def check_for_repo_name(self, name) -> bool: + """ check if name exists in repos or link exists """ if name in self.repos or os.path.exists(os.path.join('plugins', 'priv_' + name)): self.loggerr(f'name {name} already taken, delete old plugin first or choose a different name.') return False - # check if name was already initialized - if name in self.init_repos and not force: - self.loggerr(f'name {name} already initialized, not overwriting without force parameter.') - return False + return True - # overwrite existing initialized repo with same name - if name in self.init_repos and force: - del self.init_repos[name] + def create_repo(self, name, owner, plugin, branch=None) -> bool: + """ create repo from given parameters """ - repo = {} + try: + self.check_for_repo_name(name) + except Exception as e: + self.loggerr(e) + return False if not owner or not plugin: self.loggerr(f'Insufficient parameters, github user {owner} or plugin {plugin} empty, unable to fetch repo, aborting.') return False - repo['plugin'] = plugin - repo['owner'] = owner - # if branch is not given, assume that the branch is named like the plugin if not branch: branch = plugin - repo['branch'] = branch - # default to plugins repo. No further repos are managed right now - repo['gh_repo'] = 'plugins' # kwargs.get('repo', 'plugins') + repo = { + 'plugin': plugin, + 'owner': owner, + 'branch': branch, + 'plugin': plugin, + # default to plugins repo. No further repos are managed right now + 'gh_repo': 'plugins' + } # build GitHub url from parameters. Hope they don't change the syntax... repo['url'] = f'https://github.com/{owner}/{repo["gh_repo"]}.git' @@ -428,33 +436,11 @@ def init_repo(self, name, owner, plugin, branch=None, force=False) -> bool: repo['link'] = os.path.join('plugins', f'priv_{name}') repo['rel_link_path'] = os.path.join(repo['wt_path'], plugin) - repo['force'] = force - repo['plugin'] = plugin - - if os.path.exists(repo['link']) and os.path.islink(repo['link']) and not force: - self.loggerr(f'file {repo["link"]} exists and force is not requested, aborting.') - return False - # make plugins/priv_repos if not present if not os.path.exists(os.path.join('plugins', 'priv_repos')): self.logger.debug('creating plugins/priv_repos dir') os.mkdir(os.path.join('plugins', 'priv_repos')) - self.init_repos[name] = repo - return True - - def create_repo(self, name) -> bool: - - if name in self.repos or os.path.exists(os.path.join('plugins', 'priv_' + name)): - self.loggerr(f'repo {name} already exists, not overwriting.') - return False - - if name not in self.init_repos: - self.loggerr(f'repo {name} not in own data, unable to process') - return False - - repo = self.init_repos[name] - self.logger.debug(f'check for repo at {repo["full_repo_path"]}...') if os.path.exists(repo['full_repo_path']) and os.path.isdir(repo['full_repo_path']): @@ -464,20 +450,12 @@ def create_repo(self, name) -> bool: self.logger.debug(f'found repo {repo["repo"]} at {repo["full_repo_path"]}') if "origin" not in repo['repo'].remotes: - if not repo['force']: - self.loggerr(f'Repo at {repo["full_repo_path"]} has no "origin" remote set and force is not requested, aborting.') - return False - else: - try: - if not repo['repo'].create_remote('origin', repo['url']).exists(): - raise RuntimeError(f'error creating remote "origin" at {repo["url"]}, aborting.') - except Exception as e: - self.loggerr(f'error setting up remote: {e}') - return False + self.loggerr(f'repo at {repo["full_repo_path"]} has no "origin" remote set') + return False origin = repo['repo'].remotes.origin if origin.url != repo['url']: - self.loggerr(f'Origin of existing repo is {origin.url}, expecting {repo["url"]}. Aborting.') + self.loggerr(f'origin of existing repo is {origin.url}, expecting {repo["url"]}') return False else: @@ -513,29 +491,18 @@ def create_repo(self, name) -> bool: try: branchref = repo['wt'].create_head(repo['branch'], rbranch) branchref.set_tracking_branch(rbranch) - branchref.checkout(force=repo['force']) + branchref.checkout() except Exception as e: self.loggerr(f'setting up local branch {repo["branch"]} failed: {e}') return False repo['clean'] = True - if os.path.exists(repo['link']): - if repo['force']: - self.logger.debug(f'removing link {repo["link"]} as force is set') - try: - os.remove(repo['link']) - except Exception: - pass - else: - self.loggerr(f'plugin symlink {repo["link"]} exists and force not set') - return False - self.logger.debug(f'creating link {repo["link"]} to {repo["rel_link_path"]}...') try: os.symlink(repo['rel_link_path'], repo['link']) except FileExistsError: - pass + self.loggerr(f'plugin link {repo["link"]} was created by someone else while we were setting up repo. Not overwriting, check link file manually') self.repos[name] = self.init_repos[name] del self.init_repos[name] @@ -605,7 +572,7 @@ def remove_plugin(self, name) -> bool: # github API methods # - def is_repo_clean(self, name) -> bool: + def is_repo_clean(self, name: str, exc=False) -> bool: """ checks if worktree is clean and local and remote branches are in sync """ if name not in self.repos: self.loggerr(f'repo {name} not found') @@ -625,8 +592,15 @@ def is_repo_clean(self, name) -> bool: r_branch = remote.get_branch(branch=entry['branch']) l_head = local.heads[entry['branch']].commit.hexsha r_head = r_branch.commit.sha + except AttributeError: + if exc: + f = self.loggerr + else: + f = self.logger.warning + f(f'error while checking sync status for {name}. Rate limit active?') + return False except Exception as e: - self.loggerr(f'error while checking sync status for {id}: {e}') + self.loggerr(f'error while checking sync status for {name}: {e}') return False clean = l_head == r_head @@ -774,10 +748,7 @@ def create_repo_from_gh(self, number=0, owner='', branch=None, plugin='') -> boo # default id for plugin (actually not identifying the plugin but the branch...) name = f'{r_owner}/{r_branch}' - if not self.init_repo(name, r_owner, r_plugin.lower(), r_branch): - return False - - return self.create_repo(name) + return self.create_repo(name, r_owner, r_plugin.lower(), r_branch) # # general plugin methods diff --git a/githubplugin/webif/__init__.py b/githubplugin/webif/__init__.py index 2b6f2833e..9fb1c33b5 100644 --- a/githubplugin/webif/__init__.py +++ b/githubplugin/webif/__init__.py @@ -57,7 +57,7 @@ def __init__(self, webif_dir, plugin): self.tplenv = self.init_template_environment() @cherrypy.expose - def index(self, action=None): + def index(self): """ Build index.html for cherrypy @@ -65,9 +65,6 @@ def index(self, action=None): :return: contents of the template after beeing rendered """ - if action == 'rescan': - self.plugin.read_repos_from_dir() - raise cherrypy.HTTPRedirect(cherrypy.url()) # try to get the webif pagelength from the module.yaml configuration pagelength = self.plugin.get_parameter_value('webif_pagelength') @@ -107,6 +104,17 @@ def index(self, action=None): conn=self.plugin.gh._github is not None, language=self.plugin.get_sh().get_defaultlanguage()) + @cherrypy.expose + @cherrypy.tools.json_out() + def rescanDirs(self): + try: + self.plugin.read_repos_from_dir(exc=True) + except Exception as e: + cherrypy.response.status = ERR_CODE + return {"error": str(e)} + + return {"operation": "request", "result": "success"} + @cherrypy.expose @cherrypy.tools.json_out() def getRateLimit(self): @@ -177,7 +185,7 @@ def getPull(self): if b and o: return {"operation": "request", "result": "success", "owner": o, "branch": b} else: - raise Exception(f'invalid data on processing PR {pr}') + raise Exception(f'Ungültige Daten beim Verarbeiten von PR {pr}') except Exception as e: cherrypy.response.status = ERR_CODE return {"error": str(e)} @@ -243,25 +251,6 @@ def removePlugin(self): cherrypy.response.status = ERR_CODE return {"error": str(e)} - @cherrypy.expose - @cherrypy.tools.json_out() - @cherrypy.tools.json_in() - def removeInitPlugin(self): - try: - json = cherrypy.request.json - name = json.get("name") - if name is None or name == '' or name not in self.plugin.init_repos: - raise Exception(f'Plugin {name} nicht vorhanden.') - - try: - del self.plugin.init_repos[name] - except Exception: - pass - return {"operation": "request", "result": "success"} - except Exception as e: - cherrypy.response.status = ERR_CODE - return {"error": str(e)} - @cherrypy.expose @cherrypy.tools.json_out() @cherrypy.tools.json_in() @@ -280,11 +269,11 @@ def selectPlugin(self): raise Exception(f'Fehlerhafte Daten für Repo {owner}/plugins, Branch {branch} oder Plugin {plugin} übergeben.') if confirm: - res = self.plugin.create_repo(name) + res = self.plugin.create_repo(name, owner, plugin, branch) msg = f'Fehler beim Erstellen des Repos "{owner}/plugins", Branch {branch}, Plugin {plugin}' else: - res = self.plugin.init_repo(name, owner, plugin, branch) - msg = f'Fehler beim Initialisieren des Repos "{owner}/plugins", Branch {branch}, Plugin {plugin}' + res = self.plugin.check_for_repo_name(name) + msg = f'Repo {name} oder Plugin-Link "priv_{name}" schon vorhanden' if res: return {"operation": "request", "result": "success"} diff --git a/githubplugin/webif/templates/index.html b/githubplugin/webif/templates/index.html index 6c272ff7a..5ae84ccb6 100644 --- a/githubplugin/webif/templates/index.html +++ b/githubplugin/webif/templates/index.html @@ -1,11 +1,7 @@ {% extends "base_plugin.html" %} -{% set tabcount = 2 %} -{% if len(init_repo) > 0 %} - {% set tabcount = 2 %} -{% endif %} +{% set tabcount = 1 %} {% set dataSet = 'overview' %} {% set tab1title = _('Plugins in GitHub-Repos') %} -{% set tab2title = _('Zur Installation vorbereitet') %} {% block pluginstyles %} @@ -13,7 +9,7 @@ {% block buttons %} - + {% endblock buttons %} {% block headtable %} @@ -156,6 +152,7 @@ @@ -588,43 +599,3 @@ {% endblock bodytab1 %} - -{% if init_repos|length > 0 %} - -{% block bodytab2 %} -
-
Die folgenden Plugins sind zur Installation vorbereitet, aber noch nicht installiert:
- - - - - - - - - - - - - {% for plugin in init_repos %} - - - - - - - - - - {% endfor %} - -
NamePluginAutorBranchPfad (Worktree)
{{ plugin }}{{ init_repos[plugin].plugin }}{{ init_repos[plugin].owner }}{{ init_repos[plugin].branch }}{{ init_repos[plugin].full_wt_path }} - - -
-
-
-{% endblock bodytab2 %} - -{% endif %} -