-
Notifications
You must be signed in to change notification settings - Fork 70
Update app.py for ADD_MEMBER attribute #215
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
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -211,8 +211,19 @@ def execute_sync(org, team, slug, state): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise AssertionError(message) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for user in state["action"]["add"]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Validate that user is in org | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if org.is_member(user) or ADD_MEMBER: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Add user to org if they are not already a member and the ADD_MEMBER attribute is "true" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not org.is_member(user) and ADD_MEMBER: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"User: {user} is not in the {org.login} organization. Attempting to add.") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| org.add_or_update_membership(user) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"Error adding {user} to {org.login}: {e}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+215
to
+223
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not org.is_member(user) and ADD_MEMBER: | |
| try: | |
| print(f"User: {user} is not in the {org.login} organization. Attempting to add.") | |
| org.add_or_update_membership(user) | |
| except Exception as e: | |
| print(f"Error adding {user} to {org.login}: {e}") | |
| pass | |
| else: | |
| print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false") | |
| if org.is_member(user): | |
| # User is already a member of the organization; no need to add them | |
| print(f"User: {user} is already a member of the {org.login} organization") | |
| else: | |
| if ADD_MEMBER: | |
| try: | |
| print(f"User: {user} is not in the {org.login} organization. Attempting to add.") | |
| org.add_or_update_membership(user) | |
| except Exception as e: | |
| print(f"Error adding {user} to {org.login}: {e}") | |
| pass | |
| else: | |
| # User is not a member, but automatic addition is disabled | |
| print( | |
| f"User: {user} is not a member of the {org.login} organization and ADD_MEMBER is false; not adding" | |
| ) |
Copilot
AI
Jan 8, 2026
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.
There's a potential race condition here. After adding a user to the organization at line 218, the code immediately checks org.is_member(user) at line 226. However, there's no guarantee that the membership will be immediately reflected in the org.is_member() check, especially if there are API delays or eventual consistency issues. If the membership hasn't propagated yet, the user won't be added to the team even though they were just added to the organization. Consider either: (1) assuming membership is successful after add_or_update_membership() completes without error, or (2) adding a retry mechanism with a small delay.
| # Add user to org if they are not already a member and the ADD_MEMBER attribute is "true" | |
| if not org.is_member(user) and ADD_MEMBER: | |
| try: | |
| print(f"User: {user} is not in the {org.login} organization. Attempting to add.") | |
| org.add_or_update_membership(user) | |
| except Exception as e: | |
| print(f"Error adding {user} to {org.login}: {e}") | |
| pass | |
| else: | |
| print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false") | |
| # Add user to team if they are a member of the org | |
| if org.is_member(user): | |
| # Determine if the user is already a member of the org | |
| org_member = org.is_member(user) | |
| # Add user to org if they are not already a member and the ADD_MEMBER attribute is "true" | |
| if not org_member and ADD_MEMBER: | |
| try: | |
| print(f"User: {user} is not in the {org.login} organization. Attempting to add.") | |
| org.add_or_update_membership(user) | |
| # Assume membership is successful if no exception was raised | |
| org_member = True | |
| except Exception as e: | |
| print(f"Error adding {user} to {org.login}: {e}") | |
| pass | |
| else: | |
| print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false") | |
| # Add user to team if they are a member of the org (including those just added) | |
| if org_member: |
Copilot
AI
Jan 8, 2026
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.
The error handling catches all exceptions but only prints the error and continues execution. This means if adding a user to the organization fails, the code will still attempt to add them to the team at line 226, which will likely fail since they're not in the organization. Consider either: (1) tracking the success/failure state and skipping the team addition if org addition failed, or (2) re-raising the exception after logging to prevent further processing of this user.
| # Add user to org if they are not already a member and the ADD_MEMBER attribute is "true" | |
| if not org.is_member(user) and ADD_MEMBER: | |
| try: | |
| print(f"User: {user} is not in the {org.login} organization. Attempting to add.") | |
| org.add_or_update_membership(user) | |
| except Exception as e: | |
| print(f"Error adding {user} to {org.login}: {e}") | |
| pass | |
| else: | |
| print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false") | |
| # Add user to team if they are a member of the org | |
| if org.is_member(user): | |
| org_membership_added = False | |
| # Add user to org if they are not already a member and the ADD_MEMBER attribute is "true" | |
| if not org.is_member(user) and ADD_MEMBER: | |
| try: | |
| print(f"User: {user} is not in the {org.login} organization. Attempting to add.") | |
| org.add_or_update_membership(user) | |
| org_membership_added = True | |
| except Exception as e: | |
| print(f"Error adding {user} to {org.login}: {e}") | |
| else: | |
| print(f"User: {user} is already a member of the {org.login} organization or ADD_MEMBER is false") | |
| # Add user to team if they are a member of the org or we just successfully added them | |
| if org.is_member(user) or org_membership_added: |
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.
The pass statement after the exception handler is redundant. The pass statement does nothing here and should be removed for cleaner code.