Skip to content

Conversation

@ihsaan-ullah
Copy link
Collaborator

@ihsaan-ullah ihsaan-ullah commented Aug 16, 2023

@ mention of reviewers

@Didayolo @dtuantran

A brief description of the purpose of the changes contained in this PR.

Any admin of a competition was able to rerun all submissions. NOW only super admin can rerun if submissions are more than 30 otherwise competition admin can rerun

Issues this PR resolves

Checklist

  • Code review by me
  • Hand tested by me
  • I'm proud of my work
  • Code review by reviewer
  • Hand tested by reviewer
  • CircleCi tests are passing
  • Ready to merge

…han 200. Only super admin can rerun in this case.
@Didayolo
Copy link
Member

Hi Ihsan, thank you for the PR.

Sorry to come up with suggestions only now but:

  • I think we should decrease the limit, 200 is a lot of submissions, enough to clutter the queue. Maybe put something like 30.
  • Maybe we could authorize non-admin to perform this action if the benchmark is set on a private queue (queue owned by the organizer, or a queue where the organizer is added as an admin, but not public queue). What do you think?

@ihsaan-ullah
Copy link
Collaborator Author

ihsaan-ullah commented Aug 16, 2023

  • I think we should decrease the limit, 200 is a lot of submissions, enough to clutter the queue. Maybe put something like 30.

Ok this can be done

  • Maybe we could authorize non-admin to perform this action if the benchmark is set on a private queue (queue owned by the organizer, or a queue where the organizer is added as an admin, but not public queue). What do you think?

how do we know if a queue is not the public queue?

@Didayolo
Copy link
Member

Not sure if my suggestion is easy to implement or not.

There are two concepts:

  • The default queue: the one used by all benchmarks by default, when no queue is set
  • Public queues: any queue created by an user that is set to be public (then anybody is allowed to use it)

@ihsaan-ullah
Copy link
Collaborator Author

I have changed the conditions. Now user will be able to rerun only if

  • user is super user
  • user is comp admin and submissions are <= 30 (allowed on any queue)
  • user is comp admin and submissions are > 30 and user is owner/organizer of the queue (not allowed on codabench queue)

there is no condition of public or private

@ihsaan-ullah ihsaan-ullah changed the title Rerun all submissions - restrict if submissions are more than 200 Rerun all submissions - allowed for super admin and with restrictions for competition admins Aug 16, 2023
@dtuantran
Copy link
Contributor

image

Somehow on my local instance, it doesn't show the error message saying that your submissions are too much... Just a message saying that it takes hours... This message might confuses the organizer instead of telling organizer to use their own queue because there are too much submissions to run.

@ihsaan-ullah
Copy link
Collaborator Author

image

Somehow on my local instance, it doesn't show the error message saying that your submissions are too much... Just a message saying that it takes hours... This message might confuses the organizer instead of telling organizer to use their own queue because there are too much submissions to run.

that error should come from backend. Right now what you are showing is to confirm before rerunning

@ihsaan-ullah
Copy link
Collaborator Author

ihsaan-ullah commented Sep 18, 2023

@dtuantran I have added the ENV variable for the limit but haven't tested it because the PR is old and my local setup is new with latest updates for latest PRs. I let you test it if the ENV variable works. Evetything else is the same except that the variable now comes from django settings

NOTE: you many not get the error if you are super admin. Super admins are allowed to rerun as many submissions as they want.

@dtuantran
Copy link
Contributor

@dtuantran I have added the ENV variable for the limit but haven't tested it because the PR is old and my local setup is new with latest updates for latest PRs. I let you test it if the ENV variable works. Evetything else is the same except that the variable now comes from django settings

NOTE: you many not get the error if you are super admin. Super admins are allowed to rerun as many submissions as they want.

I tested with a normal user and a super admin. The normal user can not re-run all submissions, however the error message couldn't show up as we wanted.

@ihsaan-ullah
Copy link
Collaborator Author

I tested with a normal user and a super admin. The normal user can not re-run all submissions, however the error message couldn't show up as we wanted.

Can you please explain what kind of error is wanted? Is the error not appearing at all?

Also is the env var working?

@dtuantran
Copy link
Contributor

dtuantran commented Sep 19, 2023

I tested with a normal user and a super admin. The normal user can not re-run all submissions, however the error message couldn't show up as we wanted.

Can you please explain what kind of error is wanted? Is the error not appearing at all?

Also is the env var working?

Sorry for missing information, the error message didn't appear at all, the one specified in your code here

error_message = f"You cannot rerun more than {settings.RERUN_SUBMISSION_LIMIT} submissions on Codabench public queue! Contact us on `info@codalab.org` to request a rerun."

The .env needs to be cast into "int" var. I have pushed the fix, then it works.

@ihsaan-ullah
Copy link
Collaborator Author

Sorry for missing information, the error message didn't appear at all, the one specified in your code here

error_message = f"You cannot rerun more than {settings.RERUN_SUBMISSION_LIMIT} submissions on Codabench public queue! Contact us on `info@codalab.org` to request a rerun."

This is strange. Did you run collectstatic? We can check this together

@dtuantran
Copy link
Contributor

dtuantran commented Sep 19, 2023 via email

@ihsaan-ullah
Copy link
Collaborator Author

No, I didn't run it. I'll try again

Well this will fix it because error showing on front end is also new so collectstatic in needed

@dtuantran
Copy link
Contributor

dtuantran commented Sep 21, 2023

Ok, I confirm that It works, we can merge this PR.

@Didayolo Didayolo merged commit ca886b5 into develop Sep 21, 2023
@Didayolo Didayolo deleted the rerun_submissions branch September 21, 2023 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature - Asking admin when "re-run all submissions" if there are too many submissions using the default queue

4 participants