Skip to content

Conversation

@jonhealy1
Copy link
Collaborator

@jonhealy1 jonhealy1 commented Feb 23, 2024

Related Issue(s):

Description:
Removing the es-specific language from core.py makes it easier to use other databases with the api core package in the future.

PR Checklist:

  • Code is formatted and linted (run pre-commit run --all-files)
  • Tests pass (run make test)
  • Documentation has been updated to reflect changes, if applicable
  • Changes are added to the changelog

@jonhealy1 jonhealy1 marked this pull request as ready for review February 23, 2024 06:13
Copy link
Collaborator

@jamesfisher-geo jamesfisher-geo left a comment

Choose a reason for hiding this comment

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

Nice. Just one question. Does the base_url always have a trailing slash?

"rel": Relations.self.value,
"type": MimeTypes.json,
"href": urljoin(base_url, "collections"),
"href": f"{base_url}collections",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any chance that the self href could be malformed if base_url does not have a trailing slash?

Like https://stac_base_urlcollections

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's a good point. I guess urljoin probably guarantees the url will be fine either way?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok nice. Thanks for sharing

@jonhealy1 jonhealy1 merged commit ad2dc7e into main Feb 23, 2024
@jonhealy1 jonhealy1 deleted the remove_source branch February 23, 2024 18:32
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.

3 participants