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

what should the max ID length be? #224

Closed
duglin opened this issue Dec 27, 2024 · 2 comments · Fixed by #235
Closed

what should the max ID length be? #224

duglin opened this issue Dec 27, 2024 · 2 comments · Fixed by #235
Labels
AddToSpec Done v1 Required to be resolved for v1

Comments

@duglin
Copy link
Contributor

duglin commented Dec 27, 2024

I think the only spot where this might be a concern is when they appear in URLs. Do we want to ensure that URLs aren't insanely long?

5+258+126+8+11 = 408
Leaving: 1640 for PATH and 3 IDs, or 410 (max) each

This doesn't take into account query parameters and ones like ?inline and ?filter can be quite long and repeated. We can ignore$structure since /versions/vID is longer.

If we picked an arbitrary length, like 63 (to align with attribute name lengths), then that would leave 1451 for PATH, all query parameters.

Or we can say nothing and assume it's a self correcting problem.

@duglin duglin added the v1 Required to be resolved for v1 label Dec 27, 2024
@duglin
Copy link
Contributor Author

duglin commented Jan 8, 2025

Proposal: 128 for IDs

@duglin
Copy link
Contributor Author

duglin commented Jan 15, 2025

on 1/15 call: for 128 chars

duglin added a commit to duglin/xreg-spec that referenced this issue Jan 15, 2025
- typo and wording clarifications in main spec
- remove duplicate "compatibility" in schema, it's part of core attributes now
- replaced `?export` with `?compact` and just removes the dup info. Any
  inlining would need to be requested by the client
- defined `/export` to be `/?compact&inline=*,model,capabilities`
- remove ?nested
- tweaks valid chars for ID: `[a-zA-Z0-9_][a-zA-Z0-9_.\-~@]{0,127}`
- IDs must be no longer than 128 chars
- s/$structure/$details/g

Fixes: xregistry#234
Fixes: xregistry#233
Fixes: xregistry#232
Fixes: xregistry#220
Fixes: xregistry#229
Fixes: xregistry#224
Fixes: xregistry#237

Signed-off-by: Doug Davis <[email protected]>
@duglin duglin closed this as completed in 4a92f34 Jan 16, 2025
Fannon pushed a commit to Fannon/xregistry that referenced this issue Jan 23, 2025
- typo and wording clarifications in main spec
- remove duplicate "compatibility" in schema, it's part of core attributes now
- replaced `?export` with `?compact` and just removes the dup info. Any
  inlining would need to be requested by the client
- defined `/export` to be `/?compact&inline=*,model,capabilities`
- remove ?nested
- tweaks valid chars for ID: `[a-zA-Z0-9_][a-zA-Z0-9_.\-~@]{0,127}`
- IDs must be no longer than 128 chars
- s/$structure/$details/g

Fixes: xregistry#234
Fixes: xregistry#233
Fixes: xregistry#232
Fixes: xregistry#220
Fixes: xregistry#229
Fixes: xregistry#224
Fixes: xregistry#237

Signed-off-by: Doug Davis <[email protected]>
Signed-off-by: Simon Heimler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AddToSpec Done v1 Required to be resolved for v1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant