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

move the notify-crontab to its own script #2391

Merged
merged 11 commits into from
Dec 27, 2022
Merged

Conversation

szaimen
Copy link
Collaborator

@szaimen szaimen commented Aug 30, 2022

Close #2356

Signed-off-by: szaimen [email protected]

apps/smbmount.sh Outdated Show resolved Hide resolved
apps/smbmount.sh Show resolved Hide resolved
apps/smbmount.sh Show resolved Hide resolved
addons/notify-crontab.sh Outdated Show resolved Hide resolved
@enoch85
Copy link
Member

enoch85 commented Sep 1, 2022

@szaimen Thanks! Could you please fix the failing tests, and I'll finish it off.

Signed-off-by: szaimen <[email protected]>
@szaimen
Copy link
Collaborator Author

szaimen commented Sep 1, 2022

@szaimen Thanks! Could you please fix the failing tests, and I'll finish it off.

done

@enoch85
Copy link
Member

enoch85 commented Sep 1, 2022

Thanks! Will have a look tomorrow! 🥇

@enoch85 enoch85 marked this pull request as ready for review December 26, 2022 17:31
@enoch85
Copy link
Member

enoch85 commented Dec 26, 2022

Not tested, but looks good to me.

Signed-off-by: Daniel Hansson <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
Signed-off-by: Daniel Hansson <[email protected]>
@enoch85
Copy link
Member

enoch85 commented Dec 26, 2022

OK, so coding wise almost done, but the issue that remains is how to fetch the correct MOUNT_ID. I was thinking somehting like nextcloud_occ files_external:list but in NC 25 I get Command "files_external" is not defined..

Any idea @szaimen?

@enoch85
Copy link
Member

enoch85 commented Dec 26, 2022

OK, so for an idea at least:

"$(nextcloud_occ files_external:list | tail 1)" but since I don't know how the output looks, it's hard to guess the final command.

@szaimen
Copy link
Collaborator Author

szaimen commented Dec 26, 2022

OK, so coding wise almost done, but the issue that remains is how to fetch the correct MOUNT_ID. I was thinking somehting like nextcloud_occ files_external:list but in NC 25 I get Command "files_external" is not defined..

Any idea @szaimen?

Did you enable the files_external app?

@enoch85
Copy link
Member

enoch85 commented Dec 26, 2022

Did you enable the files_external app?

Haha yes. :D

@enoch85
Copy link
Member

enoch85 commented Dec 26, 2022

@szaimen Can you check the output of the above command and form a variable that will be the actual MOUNT_ID? I don't have any SMB mount installed.

Thanks!

@szaimen
Copy link
Collaborator Author

szaimen commented Dec 26, 2022

You can use a local external storage mount as example. That should have the same output...

Signed-off-by: Daniel Hansson <[email protected]>
@enoch85
Copy link
Member

enoch85 commented Dec 26, 2022

OK, grepped it from DB.

Can you please test if this works: https:/nextcloud/vm/pull/2391/files#diff-67bceb0888b398c45857e1bc41f753da1a696075ff08b9b82f8136bc6af0139aR1124-R1134

Thanks!

@@ -1121,6 +1121,18 @@ then
print_text_in_color "$ICyan" "Nextcloud crontab updated to run every 5 minutes."
fi

# Replace iNotify checker with a script instead
if crontab -u www-data -l | grep -q "files_external:notify -v"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not get the ids from the crontab? That would be a much safer way imho...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The crontab might not be true (not used, leftover, etc), but the DB is.

Also, will several mounts generate several crontab-rows?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The crontab might not be true (not used, leftover, etc), but the DB is.

However the user might not have axtivated the feature for all mounts and does not want to do so.

Also, will several mounts generate several crontab-rows?

Yes. Each mount for which the feature was activated will have its own row.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... It makes it a bit more complicated.

Maybe the best option here is to just ditch the replacement to the new way of doing it.

Signed-off-by: Daniel Hansson <[email protected]>
@enoch85 enoch85 merged commit 4327d2b into master Dec 27, 2022
@delete-merged-branch delete-merged-branch bot deleted the enh/2356/notify-crontab branch December 27, 2022 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make the files_inotify a script instead
2 participants