Skip to content

Changes for week of July 7#384

Merged
duglin merged 4 commits into
xregistry:mainfrom
duglin:July7
Jul 10, 2025
Merged

Changes for week of July 7#384
duglin merged 4 commits into
xregistry:mainfrom
duglin:July7

Conversation

@duglin

@duglin duglin commented Jul 8, 2025

Copy link
Copy Markdown
Contributor

commit 1

  • more minor edits from reviews

commit 2

  • no way to discover namecharset at this time, just docs
  • ximportresources are now transitive

commit 3

  • make endpoints.usage an array + strict enum

commit 4

  • more tweaks

From last week:

  • Jon: can client provide the xregcorrelationid on the request? Decided not to support this at this time. Perhaps if we had a better idea of the usecase that need it.
  • Jon: Why not just add a "deprecation" extension to data and let the subscribe() filter on it - instead of a dedicated "deprecation" action? Keep as is since we can't assume all subscription managers support filtering and we really really don't want to filter on "data"

duglin added 3 commits July 8, 2025 18:06
- more minor edits from reviews

Signed-off-by: Doug Davis <duglin@gmail.com>
- ximportresources are now transitive
- more typos

Signed-off-by: Doug Davis <duglin@gmail.com>
Signed-off-by: Doug Davis <duglin@gmail.com>

@jskeet jskeet left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just a few nits/suggestions.

Comment thread core/primer.md Outdated
the length of the word `count`.
Attribute names and key names are limited to 63 characters, so why are some
Group and Resource names limited to less? Because when they appear as part of
attribute names and they can be append with phrase like `url`, `count`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

phrase => phrases

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment thread core/spec.md Outdated
shared compatible model.
- Does not imply runtime validation of the claim.
- Example: `https://raw.githubusercontent.com/xregistry/spec/refs/heads/main/schema/model.json`
- Example: `http://xregistry.io/xreg/xregistryspecs/schema-v1/docs/model.json`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is this deliberately http now instead of https?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

nope - made it https again

Comment thread core/spec.md Outdated
combination defined within the same Registry. It MUST NOT reference the
same Group under which the `ximportresources` resides.
- An empty array MAY be specified, implying no Resources are imported.
- The use of `ximportresources` MAY be transitive as long as it does not

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This "MAY" feels ambiguous to me. Are we saying that the server may support transitive imports but doesn't have to, or that a particular usage may be transitive but it's valid to import a resource that doesn't import anything itself? If it's the latter, I think it would be better to say "The use of ximportresources MUST be handled in a transitive fashion [...]" or similar - to make it clear that this is really a requirement of the server.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is this any better?

- The definition of a Group MAY include an `ximportresources` directive
  that references a Resource from another Group that itself is defined
  via an `ximportresources`. However, transitive definitions of Resources
  MUST NOT result in a circular import chain.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yes, I think so. Hard to say exactly why, but it feels like it's leaving less wiggle room for servers.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(Although maybe we should say that a server MUST spot circular import chains and report an error in that case.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did think about that but decided against it because we don't go out of our way to say that for all "the client MAY" type of statements. I think it should be obvious.

Signed-off-by: Doug Davis <duglin@gmail.com>
@duglin

duglin commented Jul 10, 2025

Copy link
Copy Markdown
Contributor Author

Approved on 7/10 call

@duglin
duglin merged commit 282fca4 into xregistry:main Jul 10, 2025
2 checks passed
@duglin
duglin deleted the July7 branch July 10, 2025 16:58
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