-
Notifications
You must be signed in to change notification settings - Fork 4
Add Django REST Framework API with token management interface #16
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
base: master
Are you sure you want to change the base?
Conversation
This commit implements a complete REST API for the STDWeb project with secure token management:
Features:
- Django REST Framework integration with token authentication
- API endpoints for FITS image upload with processing options
- Task listing and detail retrieval endpoints
- Preset configuration endpoints
- Web-based API token management interface
- Automatic token generation and secure regeneration
- Comprehensive API documentation
API Endpoints:
- POST /api/auth/token/ - Get authentication token
- POST /api/tasks/upload/ - Upload FITS images
- GET /api/tasks/ - List user tasks
- GET /api/tasks/{id}/ - Get task details
- GET /api/presets/ - List available presets
Web Interface:
- /api-tokens/ - Token management page in user menu
- Copy-to-clipboard functionality
- Usage examples with actual user tokens
- Safe token regeneration with confirmations
- Integrated API documentation access
Security:
- Token-based authentication
- User isolation (users only see their own tasks)
- CSRF protection on token operations
- File validation (FITS files, 100MB limit)
- Proper error handling and HTTP status codes
Files Added:
- stdweb/serializers.py - DRF serializers
- stdweb/views_api.py - API view implementations
- stdweb/urls_api.py - API URL routing
- stdweb/views_user.py - Token management views
- stdweb/templates/api_tokens.html - Token management UI
- stdweb/management/commands/create_api_token.py - Token CLI tool
- API_USAGE.md - Complete API documentation
Files Modified:
- requirements.txt - Added djangorestframework
- stdweb/settings.py - DRF configuration
- stdweb/urls.py - Added API and user routes
- stdweb/templates/template.html - Added API Tokens menu item
…tometry: handle None detection limit; Celery: sanitize config values (convert NaN/Inf, numpy scalars) before JSON save
…otometry and transient detection
…and consulting task status
…plate subtraction)
…tometry -> subtraction with env vars
….py with venv and env vars
…pagates to inspection step
…template subtraction and update documentation
e6e984a to
167c445
Compare
|
Hi Pierre-Yves, I am extremely sorry for a long delay in reviewing your PR. I tend to accept it, but could you please first make small changes to the code? I will comment on specific places in the code below. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe I already committed the migration that covers these things (it is for adding field descriptions, so nothing really important anyway)
| ra_cen, dec_cen, sr_cen = astrometry.get_frame_center( | ||
| wcs=wcs, width=image.shape[1], height=image.shape[0] | ||
| ) | ||
| config['field_ra'] = float(ra_cen) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you planning to use these fields on client side? I do not see them used in the code proper...
I am thinking about implementing it at the level of the model itself, not as part of config, so that it will be possible to search the tasks by positions... But if you are using it already - let's keep it like that for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a bit more straightforward support for storing these values in config in d4fa84e. It does that on every run of photometry, to properly handle all possible cases (e.g. your current code actually does nothing if no blind match or refinement is performed i.e. if we rely on original astrometry from FITS header) and reduce code duplication.
| cat_col_mag1=config.get('cat_col_color_mag1'), | ||
| cat_col_mag2=config.get('cat_col_color_mag2'), | ||
| use_color=config.get('use_color', True), | ||
| force_color_term=config.get('force_color_term'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You probably need to rebase the PR against latest version of the code, so that newer changes are not removed by it
| cat_col_mag1=config.get('cat_col_color_mag1'), | ||
| cat_col_mag2=config.get('cat_col_color_mag2'), | ||
| use_color=True, | ||
| use_color=config.get('use_color', True), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this change - it is set to True intentionally here, as it is the code for "color term diagnostic" which computes and reports the value of color term for all possible filters, to see which one suits the image better. It does not impact the actual photometric calibration
| mag0 = pipeline.get_detection_limit(obj, sn=config.get('sn'), verbose=False) | ||
| config['mag_limit'] = mag0 | ||
| # Store result only if available | ||
| config['mag_limit'] = mag0 if mag0 is not None else np.nan |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to store it as np.nan and not as None as it was originally?
| verbose=sub_verbose | ||
| ) | ||
|
|
||
| # Ensure flags are 32-bit ints to avoid OverflowError during bitmask operations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed it in 77bba09 so that this change is not needed anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ModelSerializer is supposed to automatically create the fields to match the model ones, so do we need to manually specify them? I'd prefer not to duplicate the code, as it will lead to some problems later when I add new fields (and of course will forget to synchronize it to other places like this one).
More general question - do I understand it correctly that these serializations are only needed to construct JSON representation for returning in task/preset lists? Or there are some other uses, for converting something back to actual model instances?
| from . import celery_tasks | ||
|
|
||
|
|
||
| class TaskUploadAPIView(APIView): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please convert it from class-based view to function-based one, to be in line with the rest of the code?
| # --- NEW: update configuration if extra parameters are supplied --- | ||
| # Allow the same subset we accept on upload so that users can tweak | ||
| # parameters like gain/saturation before re-running an action. | ||
| extra_config_params = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not really like imposing any restriction on the fields of config, it probably should allow setting any possible value here. Again, to ease future development.
|
It seems my comments for the code somehow were invisible up till now?.. I am very sorry for that, sometimes GitHub features confuse me, sorry... |
This commit implements a complete REST API for the STDWeb project with secure token management:
Features:
API Endpoints:
Web Interface:
Security:
Files Added:
Files Modified: