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

Fixing Issue#1567 datacollector orm to api migration #1569

Merged

Conversation

MFlyer
Copy link
Member

@MFlyer MFlyer commented Dec 5, 2016

Hi @schakrava ,
had my way to #1567 following your suggestions about CRUD ops

Checking comments on commits you'll find checking pincard existence is always with direct db reading (as you suggested it possible on #1556 )

Note: Pincard view is a small one because the system/pinmanager already had all checks

Tests performed:

  • Pincard creation both with a Rockstor user and root -> OK (actually on CRUD it seems faster then before)
  • Password reset on login page both with normal user and root -> OK
  • Password reset on login simulating wrong pins and/or OTP -> OK

Ready for your review / to be merged

Mirko

Added urls to Pincard view

Signed-off-by: Mirko Arena <mirko.arena@gmail.com>
Pincard simple rfc view added to Rockstor
Our new Pincard view handles only POST requests for password reset
and Pincard creation
All read ops till under data_collector - as suggested by @schakrava
Signed-off-by: Mirko Arena <mirko.arena@gmail.com>
…Wrapper

To @schakrava
Having your suggestions on previous issue had db write ops via APIwrapper:
create_pincard and password_reset now run on APIwrapper with 2 POST requests
check_has_pincard still on ORM having only small readings (check email not enabled,
check if user pincard exist and grab 4 random pins)

Signed-off-by: Mirko Arena <mirko.arena@gmail.com>
Signed-off-by: Mirko Arena <mirko.arena@gmail.com>
@schakrava
Copy link
Member

Thank you @MFlyer. Nicely done as always!

@schakrava schakrava merged commit d911137 into rockstor:master Dec 17, 2016
@MFlyer MFlyer deleted the issue#1567_datacollector_ORM_to_api_migration branch December 18, 2016 01:31
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.

2 participants