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

Code refactoring: Remove RestService #5041

Conversation

noliveleger
Copy link
Contributor

@noliveleger noliveleger commented Jul 31, 2024

Description

ATTENTION RELEASE NOTES WRITER: the changes listed in kobotoolbox/docs#374 need to be called out in the release notes. —jnm

Part IV of #5036.
Remove Rest RestService openrosa sub app

(KPI) hooks are triggered directly when a submission comes in.

Notes

⚠️ **This PR contains a migration that alters big tables such as logger_xform and logger_instance.

Before KPI was sending a PATCH request to KoboCAT to make KoboCAT know about existence of an active Rest Service (thanks to has_kpi_hook). When a submission came in, KoboCAT sent a request to /api/v2/hook-signal endpoint to let KPI know a new submission has been added. logger_instance.posted_to_kpi was updated according to the success of the response of KPI endpoint. (but we never used it kobotoolbox/kobocat#580)

All this back and worth is useless now since everything is under the same roof.

  • Remove logger.XForm has_kpi_hook field
  • Remove logger.Instance posted_to_kpi field
  • Remove openrosa.rest_service django app
  • Remove hook signal viewset

Improvements:

Use retry_backoff and autoretry_for instead of self.retry for Celery task.

Related issues

Related to #5036
kobotoolbox/kobocat#580 (to be closed when #5036 is merged)

- Remove logger.XForm `has_kpi_hook` field
- Remove logger.Instance `posted_to_kpi` field
- Remove openrosa.rest_service django app
- Remove hook signal viewset
@noliveleger noliveleger requested a review from jnm July 31, 2024 18:10
@noliveleger noliveleger force-pushed the kobocat-django-app-part-2-remove-kobocat-rest-service branch from c64c89f to 2abd5ec Compare August 1, 2024 16:40
@noliveleger noliveleger marked this pull request as ready for review August 1, 2024 16:41
@noliveleger noliveleger force-pushed the kobocat-django-app-part-2-remove-kobocat-rest-service branch from 2abd5ec to 5943b24 Compare August 1, 2024 20:56
@noliveleger noliveleger force-pushed the kobocat-django-app-part-2-remove-kobocat-rest-service branch from 0f84555 to c8996f4 Compare August 1, 2024 22:03
@jnm jnm changed the base branch from kobocat-django-app-part-2 to kobocat-django-app-part-2-remove-kobocat-backend August 20, 2024 15:51
Copy link
Member

@jnm jnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've left some questions to discuss. not sure anything actually needs to be changed, though

kobo/apps/hook/models/hook_log.py Show resolved Hide resolved
kobo/apps/hook/models/hook_log.py Show resolved Hide resolved
kobo/apps/hook/tasks.py Show resolved Hide resolved
kobo/apps/hook/constants.py Show resolved Hide resolved
kobo/apps/hook/tests/test_ssrf.py Show resolved Hide resolved
kobo/apps/hook/views/v2/hook.py Show resolved Hide resolved
kobo/apps/hook/tests/test_api_hook.py Show resolved Hide resolved
Base automatically changed from kobocat-django-app-part-2-remove-kobocat-backend to kobocat-django-app-part-2 August 22, 2024 22:18
Copy link
Member

@jnm jnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mock IP address change thing does not need to block merging :)

@jnm jnm merged commit 944d24f into kobocat-django-app-part-2 Aug 23, 2024
4 checks passed
@jnm jnm deleted the kobocat-django-app-part-2-remove-kobocat-rest-service branch August 23, 2024 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants