-
-
Notifications
You must be signed in to change notification settings - Fork 313
Migrate User fields to MemberProfile (PR 1: Schema & Sync) #2686
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: main
Are you sure you want to change the base?
Changes from all commits
7d33b9a
e78b731
e6fb0e2
c064284
607f50f
e895d6e
818b398
2316c6d
d145a04
e9e0535
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,12 +13,10 @@ | |
| "avatar_url", | ||
| "bio", | ||
| "company", | ||
| "contributions_count", | ||
| "email", | ||
| "followers_count", | ||
| "following_count", | ||
| "id", | ||
| "is_owasp_staff", | ||
| "location", | ||
| "login", | ||
| "name", | ||
|
|
@@ -28,6 +26,13 @@ | |
| class UserNode: | ||
| """GitHub user node.""" | ||
|
|
||
| @strawberry.field | ||
| def contributions_count(self) -> int: | ||
| """Resolve contributions count.""" | ||
| if hasattr(self, "owasp_profile") and self.owasp_profile.contributions_count: | ||
| return self.owasp_profile.contributions_count | ||
| return self.contributions_count | ||
|
|
||
| @strawberry.field | ||
| def badge_count(self) -> int: | ||
| """Resolve badge count.""" | ||
|
|
@@ -83,6 +88,13 @@ def is_gsoc_mentor(self) -> bool: | |
| return self.owasp_profile.is_gsoc_mentor | ||
| return False | ||
|
|
||
| @strawberry.field | ||
| def is_owasp_staff(self) -> bool: | ||
| """Resolve if the user is an OWASP staff member.""" | ||
| if hasattr(self, "owasp_profile"): | ||
| return self.owasp_profile.is_owasp_staff | ||
| return self.is_owasp_staff | ||
|
Comment on lines
+91
to
+96
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Infinite recursion: resolver calls itself. Same issue as The pattern used elsewhere in this file (e.g., @strawberry.field
def is_owasp_staff(self) -> bool:
"""Resolve if the user is an OWASP staff member."""
if hasattr(self, "owasp_profile"):
return self.owasp_profile.is_owasp_staff
- return self.is_owasp_staff
+ return FalseHowever, this changes backward compatibility behavior. If 🤖 Prompt for AI Agents |
||
|
|
||
| @strawberry.field | ||
| def issues_count(self) -> int: | ||
| """Resolve issues count.""" | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from apps.common.models import BATCH_SIZE | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from apps.github.models.repository_contributor import RepositoryContributor | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from apps.github.models.user import User | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from apps.owasp.models.member_profile import MemberProfile | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger = logging.getLogger(__name__) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -45,15 +46,37 @@ def handle(self, *args, **options): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .values("user_id") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .annotate(total_contributions=Sum("contributions_count")) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profiles = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| users = [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for idx, user in enumerate(active_users[offset:]): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| prefix = f"{idx + offset + 1} of {active_users_count - offset}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"{prefix:<10} {user.title}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user.contributions_count = user_contributions.get(user.id, 0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profile, created = MemberProfile.objects.get_or_create(github_user=user) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if created: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profile.github_user = user | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| contributions = user_contributions.get(user.id, 0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profile.contributions_count = contributions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profiles.append(profile) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| user.contributions_count = contributions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| users.append(user) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not len(users) % BATCH_SIZE: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| User.bulk_save(users, fields=("contributions_count",)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not len(profiles) % BATCH_SIZE: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MemberProfile.bulk_save( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profiles, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fields=("contributions_count",), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| User.bulk_save( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| users, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fields=("contributions_count",), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| User.bulk_save(users, fields=("contributions_count",)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| MemberProfile.bulk_save( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profiles, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fields=("contributions_count",), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| User.bulk_save( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| users, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fields=("contributions_count",), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+65
to
+82
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Batch logic causes duplicate saves and accumulates memory. Two issues with the current batching approach:
Apply this diff to fix the batching: - if not len(profiles) % BATCH_SIZE:
+ if len(profiles) >= BATCH_SIZE:
MemberProfile.bulk_save(
profiles,
fields=("contributions_count",),
)
User.bulk_save(
users,
fields=("contributions_count",),
)
+ profiles = []
+ users = []
- MemberProfile.bulk_save(
- profiles,
- fields=("contributions_count",),
- )
- User.bulk_save(
- users,
- fields=("contributions_count",),
- )
+ if profiles:
+ MemberProfile.bulk_save(
+ profiles,
+ fields=("contributions_count",),
+ )
+ if users:
+ User.bulk_save(
+ users,
+ fields=("contributions_count",),
+ )This clears the lists after each batch save to prevent duplicate saves and control memory usage, and only saves remaining items if any exist. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also here |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| # Generated by Django 5.2.7 on 2025-11-18 17:59 | ||
|
|
||
| from django.db import migrations, models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
| dependencies = [ | ||
| ("owasp", "0065_memberprofile_linkedin_page_id"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AddField( | ||
| model_name="memberprofile", | ||
| name="contributions_count", | ||
| field=models.PositiveIntegerField(default=0, verbose_name="Contributions count"), | ||
| ), | ||
| migrations.AddField( | ||
| model_name="memberprofile", | ||
| name="has_public_member_page", | ||
| field=models.BooleanField(default=True), | ||
| ), | ||
| migrations.AddField( | ||
| model_name="memberprofile", | ||
| name="is_owasp_staff", | ||
| field=models.BooleanField( | ||
| default=False, | ||
| help_text="Indicates if the user is OWASP Foundation staff.", | ||
| verbose_name="Is OWASP Staff", | ||
| ), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # Generated by Django 5.2.7 on 2025-11-18 18:04 | ||
|
|
||
| from django.db import migrations | ||
|
|
||
|
|
||
| def copy_user_data_to_member_profile(apps, _schema_editor): | ||
| """Copy user data to member profile.""" | ||
| User = apps.get_model("github", "User") | ||
| MemberProfile = apps.get_model("owasp", "MemberProfile") | ||
| profiles_to_update = [] | ||
| batch_size = 500 | ||
| update_fields = [ | ||
| "has_public_member_page", | ||
| "is_owasp_staff", | ||
| "contributions_count", | ||
| ] | ||
|
|
||
| for user in User.objects.all().iterator(chunk_size=batch_size): | ||
| profile, _ = MemberProfile.objects.get_or_create(github_user=user) | ||
| profile.has_public_member_page = user.has_public_member_page | ||
| profile.is_owasp_staff = user.is_owasp_staff | ||
| profile.contributions_count = user.contributions_count | ||
| profiles_to_update.append(profile) | ||
|
|
||
| if len(profiles_to_update) >= batch_size: | ||
| MemberProfile.objects.bulk_update( | ||
| profiles_to_update, update_fields, batch_size=batch_size | ||
| ) | ||
| profiles_to_update = [] | ||
|
|
||
| if profiles_to_update: | ||
| MemberProfile.objects.bulk_update(profiles_to_update, update_fields, batch_size=batch_size) | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
| dependencies = [ | ||
| ("owasp", "0066_memberprofile_contributions_count_and_more"), | ||
| ("github", "0039_remove_commit_commit_repo_created_idx"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.RunPython(copy_user_data_to_member_profile, migrations.RunPython.noop), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| # Generated by Django 5.2.8 on 2025-11-26 14:23 | ||
|
|
||
| from django.db import migrations, models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
| dependencies = [ | ||
| ("owasp", "0067_memberprofile_backward_compatibility"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AlterField( | ||
| model_name="memberprofile", | ||
| name="has_public_member_page", | ||
| field=models.BooleanField( | ||
| default=True, | ||
| help_text="Whether the member's profile is publicly visible on the OWASP website", | ||
| verbose_name="Has Public Member Page", | ||
| ), | ||
| ), | ||
| ] |
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.
🧩 Analysis chain
🏁 Script executed:
cat -n backend/apps/github/api/internal/nodes/user.py | head -100Repository: OWASP/Nest
Length of output: 164
🌐 Web query:
strawberry field decorator method shadowing infinite recursion💡 Result:
Problem: if you define a Strawberry resolver method with the same Python name as a dataclass field and that method reads the attribute via self., Python will call the method again (not the stored field), causing infinite recursion.
Why it happens (short): Strawberry lets you attach resolvers as methods. A method named X and a field X collide at attribute lookup: self.X resolves to the method, so returning self.X (or accessing the attribute in a way that triggers the method) calls the method again. See Strawberry resolvers docs. [1][2]
Minimal example that causes recursion:
Fixes (pick one):
Rename the resolver method so it doesn't shadow the attribute, and expose a GraphQL name if needed:
@strawberry.field(name="name")
def resolve_name(self) -> str:
return self._name_or_other_source
(strawberry.field supports a name parameter). [2]
Use a separate resolver function and attach it to the field, avoiding method shadowing:
def get_name(root) -> str:
return root.name # root is the underlying data object
class User:
name: str = strawberry.field(resolver=get_name)
(see docs for function resolvers). [1][3]
If the field value is stored on a different attribute (e.g., _name), read that directly in the resolver to avoid self.name recursion.
References:
Would you like a corrected code example for your specific schema?
Infinite recursion in resolver: method shadows model field.
The
contributions_countmethod on line 34 returnsself.contributions_count, which recursively calls the same method instead of accessing the underlying model field. This is a known pattern collision in Strawberry GraphQL when a resolver method name matches a dataclass field.Use a cleaner fix: rename the method and expose the GraphQL field name:
Then add
name="contributions_count"to the@strawberry.fielddecorator to preserve the GraphQL schema name, or use a function-based resolver to avoid method shadowing entirely.🤖 Prompt for AI Agents