Sage-Bionetworks / data_curator

Data and metadata ingress app
Apache License 2.0
10 stars 21 forks source link

Test manifest upload #21

Closed xdoan closed 4 years ago

xdoan commented 5 years ago

Double check if the file you upload into the HTAN Uploader "Upload Annotated Metadata as a csv" is the file that is modified on Synapse. https://www.synapse.org/#!Wiki:syn20681266/ENTITY

To keep the name of the file on Shiny, I have to copy it and save it to tmp/ and I upload that file to Synapse. I want to make sure it is uploading the new file and not the one in tmp/.

jcorn427 commented 5 years ago

Hmm, it says I don't have read permission for that synapse project.

xdoan commented 5 years ago

@jcorn427 Just invited you (or I hope it is you) to the synapse team for permissions

jcorn427 commented 5 years ago

Sweet, yep, thanks!

vthorsson commented 5 years ago

@xdoan , I might need to delay the test until I get to the set up on my office computer (where I had the configuration in place), but wanted to ask : when I ran this in late August, I think I was running

milen-sage commented 5 years ago

@vthorsson (replying instead of Xengie since she's out on vacation this week until Mon next week). Yes, you were using a local version of the Synapse app; this is useful for testing purposes (e.g. it should be able to upload manifests, and interact with Synapse the same way as the app on the server does; it will also be better for pulling and testing latest updates locally since typically we push changes to the app on the server later after first testing locally).

When you click on "Upload Annotated Metadata as a csv", behind the scenes the local shiny app copies a manifest file and saves it to tmp/ and then uploads a copy of that file to Synapse; the 'check' refers to making sure that the latest manifest is also the one that ends up on Synapse.

I will test on my end as well.

vthorsson commented 5 years ago

Thanks @milen-sage. I tried the most naive thing, which was to upload a lightly edited version of files from the earlier upload (https://www.synapse.org/#!Synapse:syn20685746), using the synapse link at the start of this post

milen-sage commented 5 years ago

Thanks! Xengie has been working on the first issue and it should be resolved once we update the server app. I will check on the second issue.

A quick note: although we have deployed the server app at this link, this is still in development and not a released version. I'd expect bumping into issues until we have the first stable version, but it is very useful to have someone else other than Xengie and I test!

jcorn427 commented 5 years ago

So, it is still saying I need permission to access the generated google sheets to edit and upload them. I requested permission on the sheet itself before but revisiting this now, it looks like something more may need to be done.

milen-sage commented 5 years ago

@jcorn427 - thanks for testing! It looks like the permissions on the generated sheet are too restrictive; I'll change the permissions (which is a separate permission problem from the "do not have read permissions for this entity" on Synapse).

The current behavior re-generates a spreadsheet that matches the one that a user wants to update, but has a different url (so google requires re-authorization). We will likely also change this behavior so that the user directly updates the existing spreadsheet.

jcorn427 commented 5 years ago

oh yeah, that would be great! however, what if a user wants to generate a new spreadsheet using the same settings?

milen-sage commented 5 years ago

They can copy the sheet directly from the google sheet web interface (that copies everything including comments, notes, formatting, and sharing if they want); this creates a new url for the sheet too.

Unfortunately Google (as far as I have seen) has not exposed this copy functionality through their Sheets API.

milen-sage commented 5 years ago

@jcorn427 the default permissions on the manifest sheet have been changed (so that anyone with the link can edit), so the issue you experienced should be resolved.

jcorn427 commented 5 years ago

Great, I'll try it again.

jcorn427 commented 5 years ago

hmm, nope, still receiving the "you need permission" page when clicking on a generated link. This was the link I was given: https://docs.google.com/spreadsheets/d/1U8x4XMh5tfKk0uoVC31XNKLMvsmFHvuZNc8fg2jgZpI

milen-sage commented 5 years ago

Thanks! @xdoan did the latest PR that updates the file permissions made it to the Shiny server backend yet?

xdoan commented 5 years ago

Its in the server, but theres a bug in the newest PR that to iron out https://github.com/Sage-Bionetworks/HTAN-data-pipeline/issues/72

milen-sage commented 5 years ago

@xdoan it looks like you resolved the bug above. Just update the comment here when you've confirmed the PR containing the permissions update (so that "anyone with the link can edit") has made it to the Shiny server and @jcorn427 can test.

xdoan commented 5 years ago

should be fine now!

jcorn427 commented 5 years ago

Ok, so I was able to access the table the shiny app generated! I went ahead and stuck a line from the metadata manifest in there just to quickly test the upload and submission process, I was able to upload and validate the .csv and was also able to successfully submit it as well.

However, when I went to retrieve the previously submitted sheet, I received the error: "No previously uploaded manifest was found"

xdoan commented 5 years ago

which project and folder was this?

jcorn427 commented 5 years ago

The project I uploaded under was HCA immune cells census, and Im not sure what folder it was... Where would I find that information?

xdoan commented 5 years ago

It would be under the "Dataset" that you chose in the first tab.

jcorn427 commented 5 years ago

Ah, then it was HCA_Census_of_Immune_Cells

xdoan commented 5 years ago

Ok, that project doesn't show that you modified the manifest, and it looks like that might be because the project settings only allowed the team you are part of to download, not to edit. So I upped the permissions to Admin. Could you try again?

jcorn427 commented 5 years ago

Sure, just tried, but got the same error. Just to clarify, when I want to get a previously uploaded spreadsheet, I select my dataset on one tab, then all I need to do is switch to the "get metadata template" tab and click the link to generate previously uploaded annotations correct?

xdoan commented 5 years ago

yup, that's all you need to do, and wait because the link can take a while to generate. It would be useful for you to come over and walk me through what you're doing, or to screenshot/screenshare it because it works for me.

If you want to look into the code: could you try uploading, then going on synapse and checking to see that the file is on synapse and modified by you, and if it is could you try running this function (insert the synID of the folder where you've uploaded something).

def get_storage_manifest_path (folderID):
    syn = synapseclient.Synapse()
    syn.login()
    entity = syn.tableQuery(  ("select id, name from syn20446927 where parentId = '"+ folderID + "' " ) )
    df = entity.asDataFrame()
    if ("synapse_storage_manifest.csv" in df.loc[:,'name'].values ) == True:
        row = df.loc[df['name'] == "synapse_storage_manifest.csv"]
        manifestID = row.loc[:,'id'].values
        fh = syn.get(manifestID.item())
        path_to_file = fh.path
        return(path_to_file)
    else:
        return None
milen-sage commented 5 years ago

@jcorn427, @xdoan just added a debug error message that would surface any warning/error the storage backend would produce (the success message could have been misleading since we weren't catching various kinds of raised exceptions). Could you test again - she'll note here when that is deployed on the server, and you can test after that.

xdoan commented 5 years ago

@jcorn427 updated the success and error messages, you should get a link to the uploaded manifest if it is successful. I am not sure how long your error message will be, but I tried to make the box big (but I can't recreate it to test)

milen-sage commented 4 years ago

Closing this issue since we have run multiple tests already. Any other work items can have their own issues/epics.