feat: introduce abra config file and load abra dir from it #419

Merged
p4u1 merged 7 commits from p4u1/abra:introduce-abra-config into main 2024-07-06 14:36:32 +00:00
Member

This is the first step to introduce a configuration file for abra. The config file must be named abra.yaml or àbra.yml`. abra look for the config file in the current directory and when not found traverses the directory tree up until it is found or the home/root directory is reached.

For now there is only one setting that is made configurable: abraDir. The new logic for setting the abra dir is the following:

  1. lookup $ABRA_DIR env
  2. look for config file and take value from there
  3. $HOME/.abra as fallback

See coop-cloud/organising#303.

Todo:

  • tests
  • link to abra config issue
This is the first step to introduce a configuration file for abra. The config file must be named `abra.yaml` or àbra.yml`. abra look for the config file in the current directory and when not found traverses the directory tree up until it is found or the home/root directory is reached. For now there is only one setting that is made configurable: `abraDir`. The new logic for setting the abra dir is the following: 1. lookup `$ABRA_DIR` env 2. look for config file and take value from there 3. `$HOME/.abra` as fallback See https://git.coopcloud.tech/coop-cloud/organising/issues/303. Todo: - [x] tests - [x] link to abra config issue
decentral1se reviewed 2024-06-24 06:43:26 +00:00
decentral1se left a comment
Owner

Nice, love to see it! Ping me for a review when it's GTG 🎉

The only thing I'd mention at this point is that people do .yaml/.yml so you might want to do some sort of grep-enabled find for the file ending.... otherwise, we can 100% count on a bug report in the future 😆

Nice, love to see it! Ping me for a review when it's GTG 🎉 The only thing I'd mention at this point is that people do `.yaml`/`.yml` so you might want to do some sort of grep-enabled find for the file ending.... otherwise, we can 100% count on a bug report in the future 😆
p4u1 force-pushed introduce-abra-config from 5eb285bf82 to 7789978582 2024-06-24 09:24:06 +00:00 Compare
p4u1 changed title from config: automatic base dir when abra.yaml was found to feat: introduce abra config file and load abra dir from it 2024-06-24 09:24:58 +00:00
p4u1 force-pushed introduce-abra-config from 7789978582 to b8276f149e 2024-06-24 09:25:14 +00:00 Compare
p4u1 force-pushed introduce-abra-config from b8276f149e to 5ecde18a3d 2024-06-24 09:30:19 +00:00 Compare
Author
Member

@decentral1se Thanks for the feedback and linking to the issue! After reading the issue again I changed the logic to take the abra dir from a concrete value (abraDir) in the config file instead of setting it to the directory of the config file.
I also added some tests and added abra.yml as another valid config file name.

@decentral1se Thanks for the feedback and linking to the issue! After reading the issue again I changed the logic to take the abra dir from a concrete value (`abraDir`) in the config file instead of setting it to the directory of the config file. I also added some tests and added `abra.yml` as another valid config file name.
decentral1se approved these changes 2024-06-24 22:00:45 +00:00
decentral1se left a comment
Owner

Great stuff @p4u1! Minor comments, feel free to merge 💝

Great stuff @p4u1! Minor comments, feel free to merge 💝
@ -0,0 +11,4 @@
// LoadAbraConfig returns the abra configuration. It tries to find a abra
// configuration file (see findAbraConfig for lookup logic). When no
// configuration was was found it returns the default config.
Owner

%s/was was/was/

`%s/was was/was/`
decentral1se marked this conversation as resolved
@ -0,0 +16,4 @@
wd, _ := os.Getwd()
configFile := findAbraConfig(wd)
if configFile == "" {
logrus.Info("no config file found")
Owner

Perhaps Warnf is best? No error and coloured output?

Perhaps `Warnf` is best? No error and coloured output?
Author
Member

Changed it to debug output for now, but can always change this later

Changed it to debug output for now, but can always change this later
decentral1se marked this conversation as resolved
@ -0,0 +22,4 @@
data, err := os.ReadFile(configFile)
if err != nil {
// Do nothing, when an error occurs
logrus.Infof("error reading config file: %s", err)
Owner
https://git.coopcloud.tech/coop-cloud/abra/pulls/419/files#issuecomment-20723
decentral1se marked this conversation as resolved
@ -0,0 +30,4 @@
err = yaml.Unmarshal(data, &config)
if err != nil {
// Do nothing, when an error occurs
logrus.Infof("error loading config file: %s", err)
Owner
https://git.coopcloud.tech/coop-cloud/abra/pulls/419/files#issuecomment-20723
decentral1se marked this conversation as resolved
@ -0,0 +62,4 @@
// Abra defines the configuration file for abra.
type Abra struct {
AbraDir string `yaml:"abraDir"`
Owner

Abra.Dir? No strong feelings tho.

`Abra.Dir`? No strong feelings tho.
Author
Member

I left it for now, but am open to changing it later :)

I left it for now, but am open to changing it later :)
decentral1se marked this conversation as resolved
p4u1 force-pushed introduce-abra-config from 5ecde18a3d to d26645fbdf 2024-06-27 08:09:26 +00:00 Compare
p4u1 force-pushed introduce-abra-config from d26645fbdf to 97ae6ac267 2024-06-28 09:56:14 +00:00 Compare
p4u1 added 1 commit 2024-07-02 16:04:38 +00:00
debug
Some checks failed
continuous-integration/drone/pr Build is failing
399d430243
Author
Member

@decentral1se can you have a look at the test failure? It works for me locally

@decentral1se can you have a look at the test failure? It works for me locally
decentral1se reviewed 2024-07-03 11:43:21 +00:00
@ -0,0 +63,4 @@
})
t.Run("from config file", func(t *testing.T) {
wd, err := os.Getwd()
Owner

Maybe this is borking it? Not sure otherwise 🤔

Maybe this is borking it? Not sure otherwise 🤔
Author
Member

wasn't borking it, but removed it anyways

wasn't borking it, but removed it anyways
p4u1 marked this conversation as resolved
p4u1 added 1 commit 2024-07-06 14:20:29 +00:00
.
Some checks failed
continuous-integration/drone/pr Build is failing
4b98953289
p4u1 added 1 commit 2024-07-06 14:23:08 +00:00
.
Some checks failed
continuous-integration/drone/pr Build is failing
4791d93747
p4u1 added 1 commit 2024-07-06 14:26:42 +00:00
debug
Some checks failed
continuous-integration/drone/pr Build is failing
71f7902189
p4u1 added 1 commit 2024-07-06 14:28:55 +00:00
.
Some checks failed
continuous-integration/drone/pr Build is failing
973270b8b5
p4u1 added 1 commit 2024-07-06 14:33:46 +00:00
now
All checks were successful
continuous-integration/drone/pr Build is passing
1fbc07a2b4
Author
Member

@decentral1se there error was because the tests are run with an $ABRA_DIR set... this always overrides the config file. I unset it at the begging of the test now

@decentral1se there error was because the tests are run with an $ABRA_DIR set... this always overrides the config file. I unset it at the begging of the test now
p4u1 merged commit ac695ae28e into main 2024-07-06 14:36:32 +00:00
p4u1 deleted branch introduce-abra-config 2024-07-06 14:36:32 +00:00
Sign in to join this conversation.
No description provided.