Sage-Bionetworks / sysbioDCCjsonschemas

SysBio DCC JSON schemas
1 stars 7 forks source link

addressing issue #124 #126

Closed danlu1 closed 2 years ago

danlu1 commented 2 years ago

There are several changes in this PR:

  1. deletes output_file and output_type arguments, and utilizes the schemas.yml to extract filename and output folder.
  2. allow user to decide whether to sort the output as the json file or not.
danlu1 commented 2 years ago

One clarification question. Do we want the shared template be identical to both AD and PEC?

[image: image.png] Dan Lu(she/her) Bioinformatics Engineer/Analyst, Sage Bionetworks Cell: 206-601-6116 http://sagebionetworks.org https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fsagebionetworks.org%2F&data=04%7C01%7C%7C14a199bea8bc427c118a08d89b278d60%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C1%7C637429941506870273%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=6EoKb58z5l8vGNR2%2B%2BOQFYDUjjOMdGP6r%2BBhvNcJyx8%3D&reserved=0

On Tue, Feb 15, 2022 at 2:58 PM Abby Vander Linden @.***> wrote:

@.**** requested changes on this pull request.

I tested this out and it works great, but I think we need an option to specify which config to use, PEC or AD. When I tried updating a shared template like TMTquantitation, it successfully stored to the AD templates folder but threw an error message because I don't have write access to the PEC folder in Synapse.

— Reply to this email directly, view it on GitHub https://github.com/Sage-Bionetworks/sysbioDCCjsonschemas/pull/126#pullrequestreview-883748265, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVUKVVMW2FN4HTE4MTHOLNDU3LLBTANCNFSM5OPPMKIQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.*** com>

avanlinden commented 2 years ago

@danlu1 Yes, if the config yaml is:

metadataTemplates.assay-X:
- AD: syn123
- PEC: syn456

then the templates should be identical.

danlu1 commented 2 years ago

Gotcha. Then I think we can give each other the writing permission to the template folder since we'd like to update the shared template all together. I've granted you administrator permission to PEC template folder.

avanlinden commented 2 years ago

Oh, I also just remembered that the Synapse service account synapse-service-sys-bio-dcc-tasks-01 has access to both folders already, so we can just use that. I've been using my personal synapse credentials for debugging and forgot about the service account. Once you merge this I'll re-write the documentation in the ReadME to clarify.

Thanks for putting this together -- it looks great!