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

Cadc 9965 Update Gemini resolver to support Storage Inventory #160

Closed
wants to merge 5 commits into from

Conversation

yeunga
Copy link
Contributor

@yeunga yeunga commented Sep 10, 2021

No description provided.

Copy link
Member

@pdowler pdowler left a comment

Choose a reason for hiding this comment

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

For the failing unit tests in fits2caom2: you need to add .fits extension to the BLAST test file constants.

The CadcGeminiresolver will only ever be called for URIs with the gemini scheme, so most of the code here is extraneous. The accepted pattern is to delegate so there should be a

private final StorageResolver cadcResolver = new CadcResolver(SCHEME);

Since there is temporary code to resolve GEM archive to AD, there should also be an adResolver to delegate to.

The first thing to do in toURL is check the scheme of the incoming URI vs the SCHEME constant and throw an IllegalArgumentException if they don't match. That protects against incorrect config and seems to be missing.

Then, you simply need a check like if (uri.getSchemeSpecificPart.startsWith("GEM/")) for the temporary delegate to adResolver and otherwise delegate to cadcResolver.

@yeunga
Copy link
Contributor Author

yeunga commented Sep 14, 2021

Code changes for CADC-9965 have been submitted under CADC-9964. Please refer to #161 for all code reviews and comments. Closing this PR.

@yeunga yeunga closed this Sep 14, 2021
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.

None yet

2 participants