EBIvariation / trait-curation

A web application for manual curation of trait-to-ontology mappings, including provenance and integration with EBI SPOT stack.
Apache License 2.0
2 stars 2 forks source link

Implement the page for submitting feedback to EFO #73

Closed tskir closed 4 years ago

tskir commented 4 years ago

This should follow the UI designed in #72. On clicking the “Submit issue” button, the following sequence of events should take place:

  1. Issue is created via GitHub API in the given repository
  2. Provided the previous action is successful, all terms which were part of the submission change their status from “needs (import/creation)” to “awaiting (import/creation)”
tskir commented 4 years ago

For the actual implementation, we'll need to discuss how to authorise GitHub requests. Some options I see:

  1. Create a separate bot account for the app, and have its token as a hidden (secret) configuration variable.
  2. Make requests on behalf of the user submitting the review.

I would prefer the second approach, but it depends on how easy it is to perform GitHub authorisation on the fly. The idea in that second case is that the user (authorised to the app via Google OAuth, but not yet to GitHub) clicks on the “submit” button, and this opens a new window/tab where the auth flow continues seamlessly, the user is able to log in to GitHub, authorise the app, and then the issue is created.

joj0s commented 4 years ago

@tskir I agree the second approach would be better, I have been testing it today and we can certainly do GitHub authorization on the fly.

The only slight issue is that the Python library that is recommended by GitHub to be used as an API wrapper doesn't really document its OAuth process, so I had to do that manually by using requests and the GitHub API, and then providing the access token to the library. So it is a bit inconsistent in the sense that we use the GitHub library for the issue creation process but not the auth process, but other than that it has been working well so far.

Now regarding the spreadsheets, should they be created by the user's Google profile, or by a separate app profile?

tskir commented 4 years ago

I have been testing it today and we can certainly do GitHub authorization on the fly.

Great!

So it is a bit inconsistent in the sense that we use the GitHub library for the issue creation process but not the auth process, but other than that it has been working well so far.

It doesn't sound so bad to me. On my previous work I had to authorise a pipeline to Google Cloud App Engine, and the only way to make it work was to do some crazy combination of SDK, direct requests, and also generating some security key in advance using the Cloud Console. So in comparison, this approach looks fine :)

Now regarding the spreadsheets, should they be created by the user's Google profile, or by a separate app profile?

I see no problem with creating them with the user profile. It should be even simpler than with GitHub, since we already have the user authorised through OAuth. By the way, I suspect we would need to request some scope like “Sheets API” in order to be able to create spreadsheets. If so, then we can do that when we're first authorising the user.

joj0s commented 4 years ago

So I have created this functionality as described but there are some issues that need to be discussed about that.

tskir commented 4 years ago

I will review the PR shortly, and in the meanwhile, to answer your questions:

joj0s commented 4 years ago

where those files are stored on the server—is this a location we know and can control access to?

Yes, gspread looks for this file in the ~/.config/gspread/directory, which has to be manually created.

Do you have an idea as to why spreadsheet creation takes that long? Are the rows being e.g. inserted one by one via distinct API calls, or is something else going on?

Yes, each cell is being inserted one by one as seen below this line https://github.com/EBIvariation/trait-curation/blob/11a4e261362920808253375c6c733c46678652e7/traitcuration/traits/utils.py#L64.

tskir commented 4 years ago

We can discuss the authentication in more detail directly in the PR #85; and regarding inserting the rows—I wonder if there is a way to insert multiple with the same query? Is there support for something like this either in gspread or in Google Sheets API directly?

joj0s commented 4 years ago

I wonder if there is a way to insert multiple with the same query? Is there support for something like this either in gspread or in Google Sheets API directly?

Gspread doesn't appear to support something like that, and there was an issue asking for it, to which the maintainers replied that there is no support from the Sheets API for it either. The API itself doesn't have the most clear documentation, but I haven't found a way to insert more than a cell with a a single API call.

tskir commented 4 years ago

@joj0s What about https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/batchUpdate?

joj0s commented 4 years ago

Thanks, I hadn't found that, I was looking for row/column operations. I will try that, see if I can integrate it with gspread as well. Actually now that I searched for "batch", I found that functionality in gspread as well. I will update the code shortly.