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

Add gaia query by source id #804

Merged
merged 1 commit into from
Aug 20, 2024
Merged

Add gaia query by source id #804

merged 1 commit into from
Aug 20, 2024

Conversation

toddburnside
Copy link
Contributor

@toddburnside toddburnside commented Aug 19, 2024

I went with a simpler approach rather than using an Interpreter layer and all of that. So, it currently doesn't support things like extraFields that are in the ADQLInterpreter. If these seem important, I could augment the PR.

@mergify mergify bot added the catalog label Aug 19, 2024
Copy link
Collaborator

@cquiroz cquiroz left a comment

Choose a reason for hiding this comment

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

LGTM
Should we add a test or a helper utility to verify?

Copy link

bundlemon bot commented Aug 19, 2024

BundleMon (lucuma-catalog-tests_sjs1_3)

Unchanged files (1)
Status Path Size Limits
main.js
main.js
365.25KB -

No change in files bundle size

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

@toddburnside
Copy link
Contributor Author

Should we add a test or a helper utility to verify?

I don't see any similar tests for existing functionality. What should I be testing?

@cquiroz
Copy link
Collaborator

cquiroz commented Aug 19, 2024

We don't really have tests as calling gaia can fail. We have a few examples that are standalone apps calling gaia

@toddburnside toddburnside merged commit 4c5a83d into main Aug 20, 2024
9 checks passed
@toddburnside toddburnside deleted the gaia-source-id-query branch August 20, 2024 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants