configlet lint: Should check whether UUIDs are valid. #99
Comments
|
@petertseng I am happy it's brought up because I had seen it while doing exercism/go#894 but waited for input from others since I was away for sometime and I might be missing something. Also, created this issue in UUID dependency: mattetti/uuid#2 |
|
@petertseng @ferhatelmas the concept of a UUID is fairly new, at least the support of it within Configlet. In fact, the lint command only checks for a non-empty UUID and uniqueness across exercises and tracks. Seeing as configlet is the first line of defense for any track changes, I think validating UUIDs should be part of Configlet. But I will defer to anyone on the team who may have more insight on UUID usage and validation across Nextercism. |
|
I think formal validity of an UUID should be checked before even asking upstream for duplicates… Of course the API server should check again ;) We know this already from web forms ;) Since I already copied from wikipedia, a the canonical representation is written with 32 hexadecimal digits, grouped with a rythm of 8-4-4-4-12, therefore a quick and dirty check in configlet could be to match against a regex This should catch at least obviously invalid UUIDs. If though the package that is used to generate, is also able to parse them, I'd prefer full offline validation, depending on which version we use. |
Agreed. |
https://godoc.org/github.com/satori/go.uuid might be a good option. |
|
At least the documentation looks good. I've looked at the currently used UUID generator and its in fact only capable of generating UUIDv4 (randomly generated). And has no option to read or verify UUIDs. As a matter of fact, I'd suggest not to use plain UUIDv4. I'd generate a single UUID using v1 or v4 by @kytrinyx as a base. We could use this to namespace UUIDv3/v5 per track and then use the tracks UUID as namespace for the exercises. This way, we will still have low chances of collision, while the UUID per track/exercise combination is deterministic. I'm not sure though, how we would handle clashes then. With v4 we could simply regenerate, With v3/v5 we could switch version once, and after that do a v4, but to be honest, if we clash 2 times in a row, then we have another kind of trouble :) |
|
Are we seeing UUID clashing as an issue here? Lint will check for a clash although if using the I'm not sure what we use the UUID for in |
|
The uuid generating library is just plain broken it seems (and perhaps abandoned). If we are going by the strict definition of UUID the final group is too long and the library's own tests do not catch it. We should move to a more robust option like the one @ferhatelmas suggested or google/uuid? |
|
The UUID is used to represent the exercise, and it needs to be unique across all exercises on Exercism. I'd suggest that we use We have until v2 goes live to get the uuids right in some final/usable form. I would suggest that we switch out the dependent library, and submit PRs to fix all the tracks to fix the UUIDs. |
|
Is there a summary of what was ultimately the decision? |
|
We decided to:
This doesn't impact the v1 exercism site, but will require a full database refresh of v2 (which was already planned, so we're not losing any data that we thought we would keep around). |
|
Going back to the original title of this issue: Should we detect invalid UUIDs on CI? (I think it would be worth doing). |
|
@kytrinyx we should during the lint? We can message as to the nature of the failure and how to correct it. |
|
I agree that checking on CI is crucial, even at lint. But my reservation, and I admit that I have not thought much into the legitimacy of this problem, is that if we run this at lint we will need to change all test data to use valid UUIDs unless there is a disable flag. |
|
@nywilken Are you saying some tests for exercises may trigger a failure if this is run in lint? |
|
@Stargator I was specifically speaking about the unit tests for configlet. We currently use UUIDs like “aaa” in configlet fixtures. UUID validation in configlet should not affect tests within exercises. |
petertseng
changed the title
|
Rephrased from question to task. |
all tests that involve lint are hereby updated to do one of the two: 1. have valid UUIDs 2. keep their invalid UUIDs, and expect that an error for them is shown. Closes exercism/configlet#99
all tests that involve lint are hereby updated to do one of the two: 1. have valid UUIDs 2. keep their invalid UUIDs, and expect that an error for them is shown. Closes exercism/configlet#99
Want to prevent any invalid UUIDs from being entered. Want to put this in configlet lint, but can't: exercism/configlet#99 exercism/configlet#168 So it will go in individual tracks' .travis.yml for now. It appears that the site will accept pretty much arbitrary strings as UUIDs for now, but we want to make less work for ourselves when valid UUIDs are required.
Want to prevent any invalid UUIDs from being entered. Want to put this in configlet lint, but can't: exercism/configlet#99 exercism/configlet#168 So it will go in individual tracks' .travis.yml for now. It appears that the site will accept pretty much arbitrary strings as UUIDs for now, but we want to make less work for ourselves when valid UUIDs are required.
Want to prevent any invalid UUIDs from being entered. Want to put this in configlet lint, but can't: exercism/configlet#99 exercism/configlet#168 So it will go in individual tracks' .travis.yml for now. It appears that the site will accept pretty much arbitrary strings as UUIDs for now, but we want to make less work for ourselves when valid UUIDs are required.
Want to prevent any invalid UUIDs from being entered. Want to put this in configlet lint, but can't: exercism/configlet#99 exercism/configlet#168 So it will go in individual tracks' .travis.yml for now. It appears that the site will accept pretty much arbitrary strings as UUIDs for now, but we want to make less work for ourselves when valid UUIDs are required.
Want to prevent any invalid UUIDs from being entered. Want to put this in configlet lint, but can't: exercism/configlet#99 exercism/configlet#168 So it will go in individual tracks' .travis.yml for now. It appears that the site will accept pretty much arbitrary strings as UUIDs for now, but we want to make less work for ourselves when valid UUIDs are required.
Want to prevent any invalid UUIDs from being entered. Want to put this in configlet lint, but can't: exercism/configlet#99 exercism/configlet#168 So it will go in individual tracks' .travis.yml for now. It appears that the site will accept pretty much arbitrary strings as UUIDs for now, but we want to make less work for ourselves when valid UUIDs are required.
* travis: reject invalid UUIDs Want to prevent any invalid UUIDs from being entered. Want to put this in configlet lint, but can't: exercism/configlet#99 exercism/configlet#168 So it will go in individual tracks' .travis.yml for now. It appears that the site will accept pretty much arbitrary strings as UUIDs for now, but we want to make less work for ourselves when valid UUIDs are required.
Decision:
configlet lintshould check whether UUIDs are valid.Support:
Opposition: None.
Task: Make
configlet lintcheck whether UUIDs are valid.Original issue text/question:
What does valid mean? Is it the standard
xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxxwhere each x is a hexadecimal digit?Whatever "valid" means in the Exercism context, would it be good for configlet to check for invalid UUIDs? If whatever is using those UUIDs is expecting them all to be valid, it might be needed.
The text was updated successfully, but these errors were encountered: