Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add no-users option to DumpImporter and DumpExporter #1165

Merged
merged 5 commits into from
Nov 17, 2021

Conversation

ranaldmiao
Copy link
Contributor

@ranaldmiao ranaldmiao commented Nov 8, 2020

At the moment, when exporting a contest using cmsDumpExporter, there is no option to omit information related to users during the export.
Thus, any user which is related to the contest (participants, admin, etc..) will also be included in the dump file. When importing this dump file into another CMS instance, cmsDumpImporter will attempt to create these users. If existing users with the same username exists, this will result in database IntegrityError, causing the import to fail.

This PR introduces the option to export a contest without any user information via cmsDumpExporter. (i.e. Just the tasks itself).


This change is Reviewable

@codecov
Copy link

codecov bot commented Nov 8, 2020

Codecov Report

Merging #1165 (f41a5d8) into master (91a32fc) will increase coverage by 0.13%.
The diff coverage is 90.32%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1165      +/-   ##
==========================================
+ Coverage   63.68%   63.82%   +0.13%     
==========================================
  Files         233      233              
  Lines       17113    17131      +18     
==========================================
+ Hits        10899    10934      +35     
+ Misses       6214     6197      -17     
Flag Coverage Δ
functionaltests 47.62% <9.67%> (+<0.01%) ⬆️
unittests 44.14% <90.32%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmscontrib/DumpImporter.py 69.90% <85.71%> (+0.38%) ⬆️
cmscontrib/DumpExporter.py 73.78% <92.85%> (+1.10%) ⬆️
cms/db/util.py 66.66% <100.00%> (+13.33%) ⬆️
cms/db/base.py 87.12% <0.00%> (-2.98%) ⬇️
cms/service/workerpool.py 66.11% <0.00%> (-2.23%) ⬇️
cms/service/esoperations.py 83.33% <0.00%> (-0.70%) ⬇️
cms/service/EvaluationService.py 74.62% <0.00%> (-0.50%) ⬇️
cms/grading/Job.py 89.28% <0.00%> (-0.45%) ⬇️
cms/io/service.py 71.51% <0.00%> (ø)
cms/db/filecacher.py 79.20% <0.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91a32fc...f41a5d8. Read the comment docs.

@andreyv
Copy link
Member

andreyv commented Nov 10, 2020

Thanks.

Observe that this feature is conceptually very similar to a contest loader: there is some task data and one wants to import it. Just for clarification, do you use it for a similar purpose, e.g., prepare a contest on a staging server and import it on a testing/production server?

@ranaldmiao
Copy link
Contributor Author

Yes, this is for the purpose you mentioned. The 'staging' server as well as the testing/production server are both CMS.

At the moment, this is done by exporting the contest (using cmsDumpExporter) from the staging CMS instance and then re-importing them into the production instance using cmsDumpImporter.

Would you recommend looking into implementing a ContestLoader that reads from a cms dump exporter format?

@andreyv
Copy link
Member

andreyv commented Mar 10, 2021

Thanks for the explanation.

I think the "recommended" and more robust solution would be to prepare the contest using one of the loader formats in the first place, and import it both on the staging and production servers.

But I suppose a complementary solution like this one can be added as well, if it is useful. In this case, you should also add the same option to DumpImporter.py to keep it synchronized with DumpExporter.py.

@ranaldmiao ranaldmiao changed the title Allow DumpExporter to only export tasks Add no-users option to DumpImporter and DumpExporter Mar 20, 2021
@ranaldmiao
Copy link
Contributor Author

@andreyv Requested changes have been made, do review when you have the time.

Thanks once again :)

@andreyv
Copy link
Member

andreyv commented Nov 10, 2021

Thanks, it looks good now.

I see that you added tests and also fixed a bug that could cause DumpImporter to import everything instead of skipping items as requested — nice!

I added some minor changes in f41a5d8:

  • In DumpExporter, simplify the user-related class list and only check classes that can be reached from root (from Contest and Task classes). The other previously listed classes, including User, should not appear there.
  • In DumpImporter, also skip Announcement. Otherwise it could indirectly import Admins as well when importing a full dump with -X.
  • Whitespace and indentation fixes.

I'll merge this MR in a few days if you have no further comments.

@andreyv andreyv merged commit ed6903a into cms-dev:master Nov 17, 2021
@andreyv
Copy link
Member

andreyv commented Nov 17, 2021

Merged!

stefano-maggiolo pushed a commit to stefano-maggiolo/cms that referenced this pull request Dec 21, 2021
* feat: Allow DumpExporter to only export tasks

* fix: Dump exporter tests

* fix: Add DumpExporterTest for skip_users, fix bug

* feat: Add no-users option to DumpImporter

* fixup

Co-authored-by: Andrey Vihrov <andrey.vihrov@gmail.com>
RezwanArefin01 pushed a commit to RezwanArefin01/cms that referenced this pull request Dec 27, 2021
* feat: Allow DumpExporter to only export tasks

* fix: Dump exporter tests

* fix: Add DumpExporterTest for skip_users, fix bug

* feat: Add no-users option to DumpImporter

* fixup

Co-authored-by: Andrey Vihrov <andrey.vihrov@gmail.com>
zj-cs2103 pushed a commit to zj-cs2103/cms that referenced this pull request Mar 20, 2022
* feat: Allow DumpExporter to only export tasks

* fix: Dump exporter tests

* fix: Add DumpExporterTest for skip_users, fix bug

* feat: Add no-users option to DumpImporter

* fixup

Co-authored-by: Andrey Vihrov <andrey.vihrov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants