bioimage-io / bioimageio-uploader

1 stars 2 forks source link

UI Improvements #65

Open oeway opened 4 months ago

oeway commented 4 months ago
FynnBe commented 4 months ago

For published models, only the maintainers can upload new versions, otherwise it's another model

IMO this puts too much workload onto maintainers. updating existing models is a common scenario that any uploader should be able to initiate. I am planning to add warnings for the reviewer, e.g. if authors or citations have been removed in a revised version, etc. But we should not force uploaders to clone their resource for minor changes (or have to do it for them.. who has time for that??)

Prevent uploading the same model if there is a staged version, this will limit to only one staged version on the frontend (@FynnBe Please look into this, we need also to implement this on the collection)

This does not make sense IMO. If changes were requested a user should be able to upload another staged version without the currently staged version vanishing... @oeway you asked for some sort of history to be preserved at some point in the discussion, so this 180 is a surprise. I think keeping the staged version history (at least until one staged version is published) makes a lot of sense. Also uploaders may upload two variants if they are not sure about a certain field, etc and then a maintainer can simply accept the "right one". Restricting staged versions to a single staged version has too many limitations.

In the workflow of the collection CI, write the run_id (https://docs.github.com/en/actions/learn-github-actions/variables) to staged log, then get it from the client, so we can display it to the reviewer for the CI status.

very nice idea! can we generate a badge like button then?? maybe we should incorporate this into the status (version.json) instead? @jmetz what would be more convenient for you?

Log on the status page, 1 level of collapse

possibly (per default) only showing the last log entry per category (the 'latest' log entry), what do you think @oeway @jmetz ?

In the log details display more error info

👍

For the accept button, we will solved the bug by one staged version.

what bug?

FynnBe commented 4 months ago

IMO this puts too much workload onto maintainers. updating existing models is a common scenario that any uploader should be able to initiate. I am planning to add warnings for the reviewer, e.g. if authors or citations have been removed in a revised version, etc. But we should not force uploaders to clone their resource for minor changes (or have to do it for them.. who has time for that??)

Oh sorry. I misunderstood. I thought you meant bioimage.io maintainers, not model maintainers. Yeah m that makes a lot of sense. I'll put that limit in backoffice. @jmetz please adjust the frontend accordingly. Let's chat about it if you need additional backend support for this. Maintainers need to be read out from the rdf currently. And bioimageio maintainers should of course also always be eligible.

oeway commented 4 months ago

Oh sorry. I misunderstood. I thought you meant bioimage.io maintainers, not model maintainers. Yeah m that makes a lot of sense. I'll put that limit in backoffice. @jmetz please adjust the frontend accordingly. Let's chat about it if you need additional backend support for this. Maintainers need to be read out from the rdf currently. And bioimageio maintainers should of course also always be eligible.

Right! Exactly, so maybe call bioimage.io admin instead of maintainers.

This does not make sense IMO. If changes were requested a user should be able to upload another staged version without the currently staged version vanishing... @oeway you asked for some sort of history to be preserved at some point in the discussion, so this 180 is a surprise. I think keeping the staged version history (at least until one staged version is published) makes a lot of sense. Also uploaders may upload two variants if they are not sure about a certain field, etc and then a maintainer can simply accept the "right one". Restricting staged versions to a single staged version has too many limitations.

I think it's good to keep the staged version history, however, there should be only one active staged version (the latest one), once a new staged version is uploaded, all the historical staged versions should be locked -- this is not the case at the moment, reviewers can review and accept any of the staged versions which is confusing and causing trouble for publishing. Some where, either frontend or backend, should limit the actions, and make sure there is only one active staged version.

Also keep in mind, only the model maintainers (typically 1 person) who is uploading the staged versions for the same model, and the bioimage.io admin will upload new staged versions. So it doesn't make sense to maintain parallel variants of staged versions, they should be all follow a lineage with strict inherence relationship. The frontend should make sure the uploader are informed about previous version of staged versions, and the latest one becomes active.

possibly (per default) only showing the last log entry per category (the 'latest' log entry), what do you think @oeway @jmetz ?

We are thinking about a very brief summary of logs, and delegate the details in the github action logs.

For the accept button, we will solved the bug by one staged version. what bug?

The bug is also mentioned above, right now we can accept any staged version at any time. I think all the historical staged versions should be locked.

@jmetz @FynnBe Also, I am thinking that since we will always only have 1 active staged version (if @FynnBe agrees), it doesn't make sense to have separate status page for every staged versions, could you maybe merge all the information about a model into 1 status page? Like that, one same page, we should a full history (collapsed) of all the staged version, published version, logs for each version, and also chat history for all the versions. Then there will be only 1 status page for each model, and this can also be directly embedded into the bioimage.io model info page.

FynnBe commented 4 months ago

oh I see now what you had in mind... yes, superseding staged versions by newly staged versions makes sense and does simplify the process 👍 I'll add that to backoffice currently logs["bioimageio.spec"] will show a list of validation summaries. each run (with a new spec lib version etc) generates a new entry. it is important to reevaluate models, but at the same time this generates a lot of redundant information. we cannot simply overwrite as this history is also good to have. If it makes things easier for the frontend I can add a "latest_logs.json" with the latest log entries of each category respectively. would that be useful @oeway @jmetz?

FynnBe commented 4 months ago

Re: chat (default to last version)

this would mean users end up in a chat expecting their old messages, maybe answers... but it's empty as a new version came.. so maybe we can solve this a bit more elegantly by implementing (simple) discussions: I made a new issue for that: https://github.com/bioimage-io/collection/issues/55

oor.. we try a more proper, existing solution like matrix rooms, etc.. ? we shouldn't reinvent a chat program

FynnBe commented 4 months ago

to validate if an uploader is eligible to upload a new version I have to check the uploder.email (meaning that email needs to be present in the maintainer/author entry (which is an optional field) or the previous uploader.email of course. Fine. But to check for admin privileges the easiest would be to check the uploader email as well... Are you ok with adding a public email (the one you would use for uploads) to the reviewers.json to perform this check? @oeway , @jmetz which email would that be for you?

oeway commented 4 months ago

Yes, fine with me for the email (oeway007@gmail.com), I have it for all my pypi packages too.

If it makes things easier for the frontend I can add a "latest_logs.json" with the latest log entries of each category respectively. would that be useful @oeway @jmetz?

Since there won't be any parallel active staged or published versions, would it make sense to have a single summary log file and we always append information to the same summary log file (with links to github actions runs or potentially detailed log files).

Re: chat (default to last version) this would mean users end up in a chat expecting their old messages, maybe answers... but it's empty as a new version came..

Not really, on the frontend, all the chat history are stitched together by message creation time. It will be the same as a github PR page, when a new version comes, we just label the new version as a message inserted into the same message list.

FynnBe commented 4 months ago

Since there won't be any parallel active staged or published versions, would it make sense to have a single summary log file and we always append information to the same summary log file (with links to github actions runs or potentially detailed log files).

having it per resource version is a nice way to keep order and intuitively ignore outdated log entries. I would definitely keep a log per version. and there are parallel active published versions.

, all the chat history are stitched together by message creation time.

👍

FynnBe commented 4 months ago

from the backoffice side all tasks are done I think