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

Feature/repo refresh option #251

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mirisbowring
Copy link

Hi,

coming from #247 (comment)

I implemented an environmental variable that enables / disables (by default) an AUTO_REFRESH option.
If this option is enabled, each ssh forced commands get appended a script to call the warehouse api to fetch the latest details about the repo.

This approach is better than any cronjob since it get's executed only if a borg activity has happened (not just on a timely basis).

I hope this get pulled since i expect this to be a useful feature for others also.

@mirisbowring
Copy link
Author

@Ravinou what do you think about this? :)

@Ravinou
Copy link
Owner

Ravinou commented Jul 20, 2024

Thank you for this contribution, which I am studying. Could you start by squashing your commits please and add the conventional scopes ?
It'll make it easier for me to read 😊. By the way, you can rebase your branch on main, I added husky to help with that recently 👍

Next, I'll have to take the time to study and test this, as you're touching on core functions, notably the repository creation shells and their regexes.

I still have my doubts about reintegrating the triggering of a task like this into borgwarehouse. Originally, v1 had a built-in cron. But it was removed because it was causing problems and nobody recommends doing that. This job should be given to another service. I'll study your PR, I promise, but I don't know if I'll approve it yet. Thanks in any case.

@mirisbowring
Copy link
Author

I will rebase :)

Wouldn't it be easier to squash the commits here on the PR and i format the PR Text according to the conventional scopes? Then the newly created (squashed) commit by GitHub will be as you wish?

Or do you want to fast forward merge to preserve original signatures?

@Ravinou
Copy link
Owner

Ravinou commented Jul 21, 2024

You can do only one commit "feat:", I will study it by merging it in a specific branch. Thanks.

- Added an env var: AUTO_REFRESH to .env.sample (defaults to false)
- Added helper script helpers/shells/cronjob.sh to call the
  borgwarehouse api for refreshing the repo status and used storage
- Added function to add this feature when creating new repositories (if
  enabled) in helpers/shells/createRepo.sh
- Added function to enable/disable this option during startup (depends
  on the configured bool for AUTO_REFRESH) in docker/docker-bw-init.sh.
	- This basically inserts or removes an additional command per
	  forced-ssh-command that calls the previous created
	  "cronjob.sh" helper script.
@mirisbowring mirisbowring force-pushed the feature/repo-refresh-option branch from de6bfc0 to 50a33cb Compare July 21, 2024 10:51
@mirisbowring
Copy link
Author

Hi,
I rebased and squashed the changes.
This change is still a bit different from v1 because it is not using any cron. it is just an additional ssh-forced-command that will be executed as soon as the borg command finishes (independend of success or failure since it does not iterfere in this case).

@Ravinou
Copy link
Owner

Ravinou commented Jul 22, 2024

Thanks, it's much clearer. I'm currently working on token API integration. As soon as it's finished, I'll start working on your proposal in the next few weeks. Thank you for your patience.

@mirisbowring
Copy link
Author

Hi @Ravinou I saw your latest relase. Before i invest time into solving the conflicts - are there chances that this PR get's merged?

@Ravinou
Copy link
Owner

Ravinou commented Sep 7, 2024

Hi, for the moment don't do the rebase, I'll have a look when I get the time back ;)

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