-
Notifications
You must be signed in to change notification settings - Fork 21
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
load projects and samples #114
Conversation
TableName: this.tableName, | ||
}; | ||
const dynamodb = createDynamoDbInstance(); | ||
const response = await dynamodb.scan(params).promise(); |
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.
This scan will need to be over the experiments table (and only getting projectId
), then with the projectIds you find there you'll need ot get the projects from projects table (some of the experiments won't have projectId
s so you could deal with that by adding a FilterExpression
with attribute_exists
in the params
).
For loading projects based on the projectIds you got from experiments you could use BatchGetItem
https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/DynamoDB.html#batchGetItem-property,
also keep in mind that the experiments table has projectId
field while projects table has projectUuid
, so there's a decision here, either using projectId
and changing the projects and samples tables or projectUuid
and changing the experiments table. (or just doing a _.transform()
between the result of the experiments
scan and the projects
batch get to change projectId
-> projectUuid
and leave this matter for some future refactoring) .
Looking at Data Management page in the staging link, I am not sure if I can see information for experiments/projects from the |
I made some fixes because of the merge conflics, check again after the build finishes |
Samples cannot be fetched due to the permissions issue described here - https://biomage.atlassian.net/secure/RapidBoard.jspa?rapidView=1&projectKey=BIOMAGE&modal=detail&selectedIssue=BIOMAGE-966 |
@StefanBabukov Marcell just resolved the permissions issue for samples for your staging link, it should be working now |
I tried to create a project and am getting |
I will do some refactoring on this code as it has some idiosyncrasies compared to the rest of the code base. i will try to fix the error you are seeing as well @cosa65 . |
This should be in a ready state at this point. I need some unit tests for this that I'll write, but the code itself should be good as is. |
const items = response.Items; | ||
|
||
if (response.Items) { | ||
if (items) { | ||
const prettyResponse = response.Items.map((item) => convertToJsObject(item)); | ||
return prettyResponse; | ||
} |
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.
why not just keep line 32 as response.Items
? that's how it's also used in the next line
On Thu, 20 May 2021 at 17:14, Marcell Pék ***@***.***> wrote:
Overall looks good, see the comments on the review.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#114 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/APTCL63X7727J7CQ6SUFU2DTOURO3ANCNFSM4463FZZA>
.
Marcell, do you need more reviews on this one?
|
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.
I am approving and merging this and we will do more unit tests and address the batch problem as part of other tickets that I will create just now
* Fixed configs for develop (#106) * change default-config values for new staging develop env * Bump hosted-git-info from 2.8.8 to 2.8.9 (#108) Bumps [hosted-git-info](https://github.com/npm/hosted-git-info) from 2.8.8 to 2.8.9. - [Release notes](https://github.com/npm/hosted-git-info/releases) - [Changelog](https://github.com/npm/hosted-git-info/blob/v2.8.9/CHANGELOG.md) - [Commits](npm/hosted-git-info@v2.8.8...v2.8.9) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump lodash from 4.17.19 to 4.17.21 (#104) Bumps [lodash](https://github.com/lodash/lodash) from 4.17.19 to 4.17.21. - [Release notes](https://github.com/lodash/lodash/releases) - [Commits](lodash/lodash@4.17.19...4.17.21) Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Get data upload status (#105) * create projects and samples * update projects and samples * delete projects and samples * Add FastMNN and No integration methods (#107) * Worker failing to restart (#111) * Debug logging on helm command that launches worker * Get helm status * Dsiplay helm history on upgrade failure * Do not run helm update concurrently * fix async-lock * Cleanup * Better logging * Run npm audit fix because dependabot is complaining again (#115) * Launch new experiment (#113) * Store new experiment in DynamoDB * Add type to data processing report (#117) Co-authored-by: Martin Fosco <[email protected]> * Add end-to-end authentication and authorization (#110) * Add authentication check for API * fix * Add authorization to the API * add authorization to socketio * fix existing unit tests and add a new type of test * Add tests for authentication * fix-typo * Fix documentation * learn to spell unauthenticated * debug * more-debug * Suppress missing X-Ray context errors for auth middleware lookups * fix-error * Fix canWrite type issues * Throw if experiment does not have rbac_can_write * Fix broken merge * Remove Xray noisy log * Search for noisy Xray log * Try to convince setContextMissingStrategy not to log * Fix non-context Xray traces * Remove debugging logs * Move some commenst from PR to code Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Xavier Verges <[email protected]> * Add cognito user and admin details to rbac can write (#119) * Add authentication check for API * fix * Add authorization to the API * add authorization to socketio * fix existing unit tests and add a new type of test * Add tests for authentication * fix-typo * Fix documentation * learn to spell unauthenticated * debug * more-debug * Suppress missing X-Ray context errors for auth middleware lookups * fix-error * Fix canWrite type issues * Throw if experiment does not have rbac_can_write * Fix broken merge * Add create experiment endpoint to avoid error message experiment not found when creating new experiment * Add user permissions to saved experiment * Add adminArn * Clear out repetitions before setting rbac_can_write * Remove extra console logs * Test fixes * Add 401 and 403 responses to enabled options * Rename new auth middleware * Move adminArn into config Co-authored-by: Marcell Pek <[email protected]> Co-authored-by: Marcell Pék <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Xavier Verges <[email protected]> * load projects and samples (#114) * redoing the ticket * fix * fixes * fixes * comment fixe * fixes * move to inside function * change * Refactor code to work better with async/await * Make sure keys returned to getProjectsFromIds are unqiue. * Convert projectIds into an Array * Fix conversion error between keys * Add array spreading to other reference to projectIds * Add array spreading to other references to projectIds * Make module to filter out orphaned experiments more explicit * Remove based on items Co-authored-by: Marcell Pek <[email protected]> * Recover settings accidentally removed (#121) * api to load experiments (#126) * created gem2s endpoint (#109) * created gem2s endpoint * added skeleton parameter to state-machine-definition tests * passing all required params * fixed qc-pipeline skeleton * fixed gem2s pipeline skeleton name * renamed ARN to avoid clashes * converted gem2s pipeline steps into normal ones instead of maps * added last steps for gem2s * removed testing clutter * fix manifest grep * fixed some tests * using personal IAC to bypass runner name temporarily * added debug logs * removed old comment * added temporarily pipeline config until the updated one is deployed to iac * added authorization to new endpoints Co-authored-by: Alex Pickering <[email protected]> Co-authored-by: Martin Fosco <[email protected]> * Address comments pr 126 (#128) * add route test * add test for getListOfExperiments * Save gem2s pipeline handle (#120) * created gem2s endpoint * added skeleton parameter to state-machine-definition tests * passing all required params * fixed qc-pipeline skeleton * fixed gem2s pipeline skeleton name * renamed ARN to avoid clashes * converted gem2s pipeline steps into normal ones instead of maps * added last steps for gem2s * removed testing clutter * fixed some tests * Save gem2s pipeline handle Co-authored-by: Pol Alvarez <[email protected]> Co-authored-by: Martin Fosco <[email protected]> * Get projects test (#123) * test for projects route * write tests for projects * rewrite AWS mock * Handle gem2s response (#127) * created gem2s endpoint * added skeleton parameter to state-machine-definition tests * passing all required params * fixed qc-pipeline skeleton * fixed gem2s pipeline skeleton name * renamed ARN to avoid clashes * converted gem2s pipeline steps into normal ones instead of maps * added last steps for gem2s * removed testing clutter * fixed some tests * Save gem2s pipeline handle * Save response to dynamodb * Add gem2s handle and status report to status * Fix dynamodb updates to experiments and other minor things * Fix tests and minor bugs * Minor fixes Co-authored-by: Pol Alvarez <[email protected]> Co-authored-by: Martin Fosco <[email protected]> * Require auth for saving and loading projects (#124) * require auth for saving and loading projects * api to load experiments * add auth to routes * Request GEM2S status from the backend (#118) * add gem2s when requesting backend status * create routes, add tests * change from pipelines to backend-status * remove package-lock.json * Delete project delete experiment [BIOMAGE-948] (#131) * delete experiments * add experimentDelete test * add checks for empty experiment * BIOMAGE-792 Return only experiments and projects that a user has access to after logging in (#122) * Only list projects with access * Check for missing user. No exception on empty list * Fixed query * Accpet empty arrays * Fix length check * Restore change lost on merge * Update tests * Fix transition between gem2s and data processing (#130) * Fix discrepancies in qc and pipeline renaming * Fix various bugs * When no samples, change 500 for 404 * Implement xaviers fix on statusResToSend * Change updateExperiment to work by doing one single request * Refactor changes in previous commit * Fix qc/pipeline discrepancy problem on work requests send * Change names to more readable options * Rename helpers * Minor rafctoring based on agi comment Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Xavier Verges <[email protected]> Co-authored-by: Martin Fosco <[email protected]> * Handle gem2s response refactoring (#129) * Adapt response swagger validation * Add missing change from status to taskName Co-authored-by: Martin Fosco <[email protected]> * Hide some wrong xray missing context logs + typos (#133) * fix nonexistent property access (#137) * [BIOMAGE-1037] - Pass metadata to gem2s (#135) * pass metadata to gem2s * exclude metadata if empty * [BIOMAGE-1036] - Run QC after GEM2S (#134) * run qc after gem2s * add test * await for pipeline runs * [BIOMAGE-1009] Fix error sns messages (#136) * Adapt code in api validation and handler in gem2s to new structure * Fix response handlers * Minor fixes and refactoring * Update test snapshots * Fix indentation * Fix validation swagger files * Fix tests Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Martin Fosco <[email protected]> * convert to async/await (#141) * Beter 404 for plots-tables (#140) * Beter 404 for plots-tables * Friendlier logging of expected errors * Broken if omg * Better logging * Log hook execution * Wrong condition, again :-( * Take care of falsy parsedMessage * Sanitize metadata key (#142) * [BIOMAGE-1027] Add safeBatchGetItem that divides batchGetItem in groups of requests of 100 keys (#138) * Add safeBatchGetItem * Add tests * Add explanatory comments * Rewrite safeBatchGetItem to work with limit of 100 for total of items instead of per table * Minor bugfix and add more unit tests * Remove obsolete object Co-authored-by: Martin Fosco <[email protected]> * [BIOMAGE-989] - Work request throw error if pipeline has not yet succeeded (#139) * new flow for releases (#143) * added deployment for release/hotfixes PRs to master (#145) * using tokens to allow flow to trigger testing * building releases/hotfixes on PR instead of push * Update ci-develop.yaml * Delete files in s3 for deleted samples (#146) * Add deletion of files of deleted project * Add basic delete single sample operation (still needs updates) * Remove samples unnecessary ids field * Fixes and add validation * Fix tests * Fix tests and cleanup code * Reorder * Get sample files instead of hardcoded files * Fix tests Co-authored-by: Martin Fosco <[email protected]> * notifying worker of job done (#151) * notifying worker of job done * Fix louvain cellsets storage in s3 * Add small comment * clean up Co-authored-by: Martin Fosco <[email protected]> * [BIOMAGE-1044] Add a needsRunning flag to gem2s pipeline status (#147) * Add needsRunning flag to gem2s status Adds a paramsHash to the gem2s handle * Minor readibility improvements and cleanup * Refactoring and fix issue with taskparams hash * Refactoring and fix tests * Fix errro during run * Refactor fix Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Pol Alvarez <[email protected]> * Deprecated change for ci.yaml Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Anugerah Erlaut <[email protected]> Co-authored-by: Juanlu <[email protected]> Co-authored-by: Xavier Vergés <[email protected]> Co-authored-by: ivababukova <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Marcell Pék <[email protected]> Co-authored-by: Marcell Pek <[email protected]> Co-authored-by: StefanBabukov <[email protected]> Co-authored-by: Alex Pickering <[email protected]> Co-authored-by: Martin Fosco <[email protected]> Co-authored-by: Martin Fosco <[email protected]>
[BIOMAGE-2111] - Use selected cell sets in Trajectory Plot
Background
Link to issue
Link to staging deployment URL
https://ui-again2-ui258-api114.scp-staging.biomage.net/data-management
Links to any Pull Requests related to this
API - #114
UI - hms-dbmi-cellenics/ui#258
Anything else the reviewers should know about the changes here
Changes
Code changes
Definition of DONE
Your changes will be ready for merging after each of the steps below have been completed:
Testing
To set up easy local testing with inframock, follow the instructions here: https://github.com/biomage-ltd/inframock
To deploy to the staging environment, follow the instructions here: https://github.com/biomage-ltd/biomage-utils
Documentation updates
Is all relevant documentation updated to reflect the proposed changes in this PR?
Approvers
Just before merging:
unstage
script in here: https://github.com/biomage-ltd/biomage-utils is executed. This script cleans up your deployment to stagingOptional