Skip to content

more and clearer error/debug messages, allow A-Z and -_. in config filenames #4

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

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

Conversation

morgana2313
Copy link

I had quite some trouble figuring out what was wrong with my csync2 setup using NAT, so I added more clear error and debug messages to figure out what;s going on.

The paper.pdf might also be clearer on the host syntax: hostname@interfacename can be misleading, hostname@alternative_NATted_hostname might help people.

The restrictions on the config names lead to hard to read confignames if you have a lot of hierarchic files. Adding A-Z -_. makes things easier to read.

Copy link
Contributor

@lge lge left a comment

Choose a reason for hiding this comment

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

usually, if you think you want to use "config names", you really want to use "groups" (and the default config name) instead.
That said, config names must be valid filenames, AND valid database names in the supported databases, and the way the database names are constructed (from myhostname and cfgname) must not lead to ambiguities.
Relaxing this from [a-zA-Z0-9] to [a-zAZ0-9_.-] looks harmless enough.

The %s -> [%s] is a matter of taste, I guess.

The indentation of the {} in your changed if () {} is slightly broken.

Trailing whitespace cleanup is fine with me ;)

Version bump is likely a good idea anyways... just to show we are still alive :-)

Looks good to me, give me a bit to think about possible "bad" implications of the less restrictive cfgname, there may be some weird side effect somewhere, I seem to remember that there was a reason to be that strict; I just don't recall what it was :-/

Anyways, should be merged soon.

Thank you for taking the time to prepare this pull request.

@morgana2313
Copy link
Author

morgana2313 commented Apr 3, 2018 via email

@morgana2313
Copy link
Author

I'm using csync2 in combination with lsync (to only start it when an inotify happens) in a dynamic environment where the config files are generated by Saltstack.

There is a valid case where peers change or might not be resolvable or online (yet). I seems that csync2 doesn't notice new peers until I delete te database.

Are there a options where I can tell csync2 to update the hosts in the database and don't abort the sync on a host that is not resolvable or online yet?

@lge
Copy link
Contributor

lge commented Apr 4, 2018 via email

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