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

[SDESK-7442] Migrate Events resource service to async #2150

Merged
merged 11 commits into from
Dec 10, 2024

Conversation

eos87
Copy link

@eos87 eos87 commented Dec 8, 2024

Purpose

Extend the new Event async resource service with the next methods

  • on_create
  • create
  • on_created
  • validate_event and related private methods
  • on_update
  • update
  • on_updated
  • on_deleted

What has changed

  • The methods mentioned above have been migrated from old sync service to new async one and the corresponding code has been adjusted as much as possible to be async (along with the depending internal/private methods)
  • Added new events_utils module in order to make some of the functions reusable
  • Adjusted related test to run

What's pending

Note

I will go through the docstrings of these changes and update them. If there is not time before sprint end, I will provide another small PR for that.

Thanks for reviewing!

Resolves: SDESK-7442

eos87 added 7 commits December 3, 2024 19:27
Along with another set of utils functions

SDESK-7442
Pending to fix test broken because of prod_api app context issue

SDESK-7442
SDESK-7442
Copy link
Collaborator

@MarkLark86 MarkLark86 left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple small suggestions

server/planning/events/events_service.py Outdated Show resolved Hide resolved
server/planning/events/events_service.py Show resolved Hide resolved
server/planning/events/events_service.py Outdated Show resolved Hide resolved
server/planning/events/events_service.py Show resolved Hide resolved
server/planning/events/events_service.py Outdated Show resolved Hide resolved
server/planning/events/events_service.py Outdated Show resolved Hide resolved
server/planning/events/events_service.py Outdated Show resolved Hide resolved
server/planning/events/events_service.py Outdated Show resolved Hide resolved
server/planning/events/events_service.py Outdated Show resolved Hide resolved
server/planning/events/events_service.py Outdated Show resolved Hide resolved
Copy link

@BrianMwangi21 BrianMwangi21 left a comment

Choose a reason for hiding this comment

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

LGTM!

@eos87
Copy link
Author

eos87 commented Dec 10, 2024

I will fix the test in a subsequent PR.

@eos87 eos87 merged commit 44cf909 into async Dec 10, 2024
8 of 20 checks passed
@eos87 eos87 deleted the hg/SDESK-7442-events-async-service branch December 10, 2024 15:33
@eos87 eos87 added the async label Dec 10, 2024
@eos87 eos87 added this to the 3.0 milestone Dec 10, 2024
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.

3 participants