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 db parameter to split_by_square after recent changes to fmtm-splitter #2020

Merged
merged 7 commits into from
Dec 24, 2024

Conversation

Sujanadh
Copy link
Collaborator

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation
  • πŸ§‘β€πŸ’» Refactor
  • βœ… Test
  • πŸ€– Build or CI
  • ❓ Other (please specify)

Related Issue

Describe this PR

After a recent upgrade of split by square in the fmtm-splitter, it now requires an extra db parameter as a unicode string of db url, which fixes the mentioned issue.

Screenshots

N/A

Checklist before requesting a review

[optional] What gif best describes this PR or how it makes you feel?

@Sujanadh Sujanadh self-assigned this Dec 23, 2024
@github-actions github-actions bot added bug Something isn't working backend Related to backend code labels Dec 23, 2024
@Sujanadh Sujanadh marked this pull request as draft December 23, 2024 10:00
@@ -230,6 +230,7 @@ async def task_event(db, project, tasks, admin_user):
new_event = TaskEventIn(
task_id=task.id,
user_id=user.id,
username=user.username,
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised this isn't calculated in the SQL, using the user id?

If you insert both the user id and username, then it opens the potential for inconsistency: inserting a username that does not match the user id

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah you are right , I got what you meant to say. I was trying to solve the test case failure issue, but it seems to have github action issue?

@github-actions github-actions bot added the devops Related to deployment or configuration label Dec 24, 2024
@Sujanadh Sujanadh marked this pull request as ready for review December 24, 2024 09:41
@Sujanadh Sujanadh merged commit 4a5664d into development Dec 24, 2024
5 checks passed
@Sujanadh Sujanadh deleted the fix/add-db-split-square branch December 24, 2024 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Related to backend code bug Something isn't working devops Related to deployment or configuration tests Related to automated code tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants