Skip to content

Make cronfile work because of filename error#25

Merged
raneq merged 1 commit intomasterfrom
fix/cronfiles_with_dots
Jun 21, 2019
Merged

Make cronfile work because of filename error#25
raneq merged 1 commit intomasterfrom
fix/cronfiles_with_dots

Conversation

@raneq
Copy link
Collaborator

@raneq raneq commented Jun 20, 2019

cron prohibits dots in the cron.d filenames. Therefore, we need to escape or replace them. I chose to replace dots with underscores

cron prohibits dots in the cron.d filenames. Therefore, we need to escape or replace them. I chose to replace dots with underscores
@raneq raneq requested a review from danypr92 June 20, 2019 16:46
@raneq raneq added this to the v1.2.1 milestone Jun 20, 2019
Copy link
Contributor

@enricostano enricostano left a comment

Choose a reason for hiding this comment

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

If it's cronfile complaining, why don't we do the s/./_ directly there instead of touching more generic variables?

@raneq
Copy link
Collaborator Author

raneq commented Jun 21, 2019

If it's cronfile complaining, why don't we do the s/./_ directly there instead of touching more generic variables?

Because it's the variable is not that generic (it's only used as the internal restic repo name and for this wrapper) and because the naming of the wrapper happens inside restic-role.

In the restore-to-controller taskfile we do it ourselves, but at main.yml we have no direct control over it, only through this variable.

Also, escaping of dots (odoo_coopdevs_org) is preferred over just using short domain (odoo) because of collisions.

@raneq
Copy link
Collaborator Author

raneq commented Jun 21, 2019

If it's cronfile complaining, why don't we do the s/./_ directly there instead of touching more generic variables?

Also, it's the filename of the cronfile causing the issue. I'm not sure about what you meant with "replace directly there"

@enricostano
Copy link
Contributor

Also, it's the filename of the cronfile causing the issue. I'm not sure about what you meant with "replace directly there"

Would you mind to point me to the place where we create that filename? Thanks!

@raneq
Copy link
Collaborator Author

raneq commented Jun 21, 2019

Also, it's the filename of the cronfile causing the issue. I'm not sure about what you meant with "replace directly there"

Would you mind to point me to the place where we create that filename? Thanks!

Again, it's not "us" who create the filename. See the linked tasks in my comment above and the external role who uses the "restic repo name" to create the "restic wrapper filename"

@raneq raneq merged commit bb9369e into master Jun 21, 2019
@raneq raneq deleted the fix/cronfiles_with_dots branch June 21, 2019 11:05
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