restrict suite name character set

[oliver-sanders]

For Discussion

address "suite registration" point of #2979

Create a unicode_rules module:

• Centralise unicode restrictions (e.g. suite / task names) for consistency.
• Provide user with an informative error explaining which rule an invalid name has broken.
• Allow us to auto-document text restrictions.

Questions

• Permit = character? No
• Permit % character? No
• Limit number of characters? Yes - 255
• Prohibit - and . as the first character (as implemented)? Yes
• Any other changes? No

Requirements check-list

• I have read CONTRIBUTING.md and added my name as a Code Contributor.
• Contains logically grouped changes (else tidy your branch by rebase).
• Does not contain off-topic changes (use other PRs for other changes).
• Appropriate tests are included (unit and/or functional).
• Appropriate change log entry included.
• (master branch) I have opened a documentation PR at restrict suite name character set cylc-doc#52

[hjoliver]

Answers (to Questions in issue description) (my opinion)

• Permit = and % characters?
  • I say no. IMO it's best to be very restrictive (just alphanumeric and the obvious word-separators [+_-@] OR almost completely unrestricted like Unix filenames (in which case we and users have to deal with the downstream consequences - so let's avoid that). If we allow = and % then why not everything else?
• Limit number of characters?
  • maybe impose 255 on the basis of filename length, although we can't really say how suite names might be used by users in filenames. Anything less will seem arbitrary and may annoy people.
• Prohibit - and . as the first character (as implemented)?
  • yes, for aesthetic reasons
• Any other changes?
  • can't think of any

[oliver-sanders]

Answers (to Questions in issue description) (my opinion)

If no-one objects I'm going to go with those decisions. We can easily change the suite name rules later.

In-case you wonder why I've implemented the suite name rules as a class, this is so it can be auto-documented in cylc-doc.

[sadielbartholomew]

If no-one objects I'm going to go with those decisions.

No objection from me! Do you want Hilary as second reviewer or can I review?

[oliver-sanders]

Please, go ahead!

[sadielbartholomew]

The code itself looks great & does as per the agreed spec outlined in the comments here from some testing with cylc register <suite-name>.

Before merge, there's one typo (sorry, might as well correct it) & also while reviewing I've had a thought regarding one of the questions towards the validity rules, namely on leading characters (which would be trivial to change now in this implementation given its highly maintainable nature 🎆). I agree in general on:

• Prohibit - and . as the first character (as implemented)?
  • yes, for aesthetic reasons

but besides this, a leading dot can have the functional purpose of making the suite directory hidden via the standard Unix filesystem display rules. Perhaps we could still allow this, as users may currently, or in future, want to utilise that in some way, e.g. to reduce clutter with "old" suites?

[sadielbartholomew]

Not a review comment, just pointing out something I found cool (so a review complement, I guess)! This method was new to me, so it was nice to learn a new Python feature by figuring out what its purpose was here, which was to add the rules returned from the methods applied inside RULES into the doctring of any sub-class:

>>> print(SuiteNameValidator.__doc__)
The rules for valid suite names:

* must be between 1 and 254 characters long
* can not start with: ., -
* can only contain: alphanumeric, /, _, +, -, ., @

Nice!

[hjoliver]

a leading dot can have the functional purpose of making the suite directory hidden via the standard Unix filesystem display rules. Perhaps we could still allow this, as users may currently, or in future, want to utilise that in some way, e.g. to reduce clutter with "old" suites?

That's an interesting idea @sadielbartholomew but I'm a bit concerned about making it easier for users to avoid housekeeping old suites, which has performance implications for "scan" functionality in the UI.

On the other hand, I'm generally in favor of giving users the power to blow their own foot off if they really want to.

Any other opinions on this?

[oliver-sanders]

Any other opinions on this?

I can't really think of any legitimate reasons for having a hidden suite, I think it would be somewhat confusing if ls diddn't display suites listed by cylc scan

Doing a quick FS scan we don't currently have a single suite on site named with a leading dot so I can't imagine anyone would miss this.

Interestingly, my FS scan did reveal 21 users with ~/cylc-run/.service directories (which isn't the best sign) and one .thumbnails directory (which is just confusing).

[matthewrmshin]

For sanity of those who do user support (i.e. us), I'd vote against hidden suites.

[hjoliver]

In that case, I think we can merge this as-is...