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

New field in links (method, headers, body) #1253

Merged
merged 19 commits into from
Jul 17, 2024
Merged

New field in links (method, headers, body) #1253

merged 19 commits into from
Jul 17, 2024

Conversation

emmanuelmathot
Copy link
Collaborator

@emmanuelmathot emmanuelmathot commented Sep 27, 2023

Related Issue(s): #1198

Proposed Changes:

  1. Added method, headers, body fields in links objects

PR Checklist:

@emmanuelmathot emmanuelmathot changed the title Initial new field in links New field in links (method, headers, body) Sep 27, 2023
@emmanuelmathot emmanuelmathot added this to the 1.1 milestone Sep 27, 2023
Copy link
Collaborator

@m-mohr m-mohr left a comment

Choose a reason for hiding this comment

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

This looks generally good, but a couple of thoughts (for here and partially also the API):

  • headers: Shouldn't this be strictly just be a Map<string, string>?
  • body: Shouldn't this allow anything? Why is it only an object? Why can't it be a string or an array for example?
  • Should we have a central place where we define links so that it's not so repetitive?

@emmanuelmathot
Copy link
Collaborator Author

This looks generally good, but a couple of thoughts (for here and partially also the API):

  • headers: Shouldn't this be strictly just be a Map<string, string>?
  • body: Shouldn't this allow anything? Why is it only an object? Why can't it be a string or an array for example?

I used the same spec as per STAC API to be aligned since the initial issue was about being in line with STAC API.

  • Should we have a central place where we define links so that it's not so repetitive?

I moved link object definitions from item, catalog and collection to common metadata page

@emmanuelmathot emmanuelmathot requested a review from m-mohr November 9, 2023 10:44
@m-mohr
Copy link
Collaborator

m-mohr commented Nov 9, 2023

I used the same spec as per STAC API to be aligned since the initial issue was about being in line with STAC API.

Maybe that's something we should actually bring up in the API and change there, too...

I moved link object definitions from item, catalog and collection to common metadata page

Not sure whether that's a good idea. That would need a broader change also in the schema as it implies links can now be used in Assets, Providers, Links itself, ... basically everywhere. I don't think that's intended.

item-spec/common-metadata.md Outdated Show resolved Hide resolved
@m-mohr m-mohr linked an issue May 6, 2024 that may be closed by this pull request
@emmanuelmathot
Copy link
Collaborator Author

Discussion in STAC meetup: use Map<string, *> that is also an object and thus not break STAC API spec

@emmanuelmathot emmanuelmathot self-assigned this Jul 4, 2024
item-spec/common-metadata.md Outdated Show resolved Hide resolved
item-spec/common-metadata.md Outdated Show resolved Hide resolved
@emmanuelmathot emmanuelmathot requested a review from m-mohr July 9, 2024 20:45
item-spec/common-metadata.md Outdated Show resolved Hide resolved
item-spec/common-metadata.md Outdated Show resolved Hide resolved
item-spec/common-metadata.md Outdated Show resolved Hide resolved
item-spec/common-metadata.md Outdated Show resolved Hide resolved
item-spec/common-metadata.md Outdated Show resolved Hide resolved
item-spec/common-metadata.md Outdated Show resolved Hide resolved
emmanuelmathot and others added 7 commits July 12, 2024 14:57
Co-authored-by: Matthias Mohr <matthias@mohr.ws>
Co-authored-by: Matthias Mohr <matthias@mohr.ws>
Co-authored-by: Matthias Mohr <matthias@mohr.ws>
Co-authored-by: Matthias Mohr <matthias@mohr.ws>
Co-authored-by: Matthias Mohr <matthias@mohr.ws>
@m-mohr
Copy link
Collaborator

m-mohr commented Jul 12, 2024

Looks solid, I just proposed two additional clarifications around the method. Happy to approve afterwards.

emmanuelmathot and others added 2 commits July 12, 2024 23:14
Co-authored-by: Matthias Mohr <matthias@mohr.ws>
Co-authored-by: Matthias Mohr <matthias@mohr.ws>
@emmanuelmathot emmanuelmathot requested a review from m-mohr July 12, 2024 21:16
@m-mohr m-mohr merged commit 155ccc3 into dev Jul 17, 2024
2 checks passed
@m-mohr m-mohr deleted the http-additions branch July 17, 2024 16:52
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.

HTTP additions for links
3 participants