search

unioslo / nivlheim

A system for collecting key information from all your servers and presenting it through an easy-to-use web GUI with search and browse functions. The previous generation of this system is used in production at the University of Oslo, Norway.
GNU General Public License v3.0
9 stars 7 forks source link

Change nivlheim_client regex parse in split_url #185

Closed Joan10 closed 1 year ago

Joan10 commented 1 year ago

Hello all,

First of all, I would like to thank for this software. We were looking for some kind of inventory software and this seems to fit exactly in our needs.

I installed Nivlheim in a test environment and I could not connect the clients to the server because the connection was failing. After looking into the code of the client I realized that the function split_url was not parsing our URLs properly. Seems to be because in our University we are using subdomains and the split_url functions could not handle this kind of URLs. More details in the commit message below.

I did some tests and seems to be working properly. Perhaps further test should be made to be sure.

_Nivlheim_client has a function which splits the server URL in several variables that contain the protocol, URL, port and path. If you passed a complex URL with subdomains (like example01.test.uib.cat) or without domain name the function was not able to parse them.

Replaced the regex to allow more complex domain names and changed the regex groups that are assigned to variables. The trivial case with localhost fits in this regex, so the last "if" is not needed anymore. Checked with localhost and other domains and seems to be working._

Greetings, Joan

oyvindhagberg commented 1 year ago

Hello! Glad to hear you want to use the software. We appreciate your contributions to the code base. The change you've made to the client here makes sense.

The repo has some GitHub actions that run a lot of tests, and right now there's one that fails. The failing action verifies a few things in PR's such as the version number being updated etc. Also, it seems the other actions aren't triggered. For simplicity I can try and make a commit or PR to your repo with the required changes.

Joan10 commented 1 year ago

Hello! Do you need me to do something more? Thanks for all.

oyvindhagberg commented 1 year ago

Hello! Do you need me to do something more? Thanks for all.

Hi @Joan10, no actually it's me who has to do something. Right now the problem is our GitHub workflow doesn't trigger on PR's from forked repos. I have to fix that so the workflow runs and your code gets tested. I was busy with another project last week, but hope to get it done this week.

oyvindhagberg commented 1 year ago

Hi, here's an update. My colleague got inspired by your pull request and re-wrote the entire split_url logic: https://github.com/unioslo/nivlheim/pull/188/commits/bfb6c0335b7990b95e579559cc2e4208dcf80118 I have decided to ultimately go for that solution as it is more comprehensive. But thanks for your contribution!