-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: use generics for config-loader and re-use for int test #165
Open
michaelfedell
wants to merge
37
commits into
development
Choose a base branch
from
feat/integration-test-config
base: development
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merging v3 golang package namespace change to match deployed version, so users of packages can work
Hot fixes for new release
Releasing diffraction detector which saves detector field. Also releasing permissions standardisation and documentation updates
Merge DOI changes to main
Public Access
…ch the dataset ID, will need to think about this for future imports. Also clearer printing of what was downloaded as importer runs
…y to switch between pseudo-intensity definitions based on something. Don't know what that is yet, reached out via email to releavant people
Feature/importer fixes
Added a hack to allow this G090 dataset to import, the RTT doesnt mat…
Make detector config GET work on public permission
…for sol847, in which case we load the new pseudo-intensity file
Added importer change to check if dataset ID is greater than the one …
Bug fixes, pseudo-intensity standards change
Fix test affected by importer logging output change
v3.7.0 release
Fixed pseudo-intensity check
Release 3.8.0
Update catalog-info.yaml
…ts, it imports the one with lowes t SCLK and logs this. This is a workaround for GDS delivering us badly named files as happened recently
Modified importer so in cases where it finds more files than it expec…
Removed cloudwatch logging from API and quant job runs
Mergemain
Want to load the config struct for integration tests in the same way as we do for the API (load from json file and then override based on PIXLISE_CONFIG_* environment variables). To do this, I refactored config loader functions to use generics; had to just use `T any` types :/ Then changed the integration tests to use a single configPath flag instead of a big set of os args; config-loader tests still passed though I've not yet tried building and running the actual integration tests
RyanStonebraker
approved these changes
Sep 19, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Want to load the config struct for integration tests in the same way as we do for the API (load from json file and then override based on PIXLISE_CONFIG_* environment variables). To do this, I refactored config loader functions to use generics; had to just use
T any
types :/Then changed the integration tests to use a single configPath flag instead of a big set of os args; config-loader tests still passed though I've not yet tried building and running the actual integration tests