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

nonetype handling for repo collaborators #1405

Closed

Conversation

danbrauer
Copy link
Contributor

@danbrauer danbrauer commented Dec 12, 2024

Summary

Fix for #1404

Related issues or links

Checklist

Provide proof that this works (this makes reviews move faster). Please perform one or more of the following:

  • Update/add unit or integration tests.
  • Include a screenshot showing what the graph looked like before and after your changes.
  • Include console log trace showing what happened before and after your changes.

Not applicable If you are changing a node or relationship:

Not applicable If you are implementing a new intel module:

@danbrauer
Copy link
Contributor Author

I did this very quickly just now and haven't had a chance to sanity check it on my real graph. Also have not added / updated tests.

@danbrauer
Copy link
Contributor Author

I had a small pocket of time at my desk so I wanted to share some sanity checks / screen caps, similar to the ones from original PR (#1391) where this issue was introduced.

This should demonstrate the feature (identifying direct vs outside collabs) and that it is still working as expected, especially on top of the existing integration tests still working.

Example repo with an outside collab

(note this will look different from the similar screencap in the original PR because we've done cleanup of some of the users)
Screenshot 2024-12-12 at 10 06 23 AM

Example repo with direct collabs

Screenshot 2024-12-12 at 10 09 30 AM

Counts to give a sense of all the connections in our graph

Screenshot 2024-12-12 at 10 10 10 AM

@danbrauer danbrauer marked this pull request as ready for review December 12, 2024 15:17
@danbrauer danbrauer force-pushed the gh_repo_collabs_none_handling branch 2 times, most recently from 17e5d38 to 468a0da Compare December 12, 2024 15:40
@danbrauer
Copy link
Contributor Author

@achantavy this is now updated with new unit testing. It should be good to go, but, I raced through this in tiny pockets of time.

@danbrauer danbrauer force-pushed the gh_repo_collabs_none_handling branch from 468a0da to 1ba368b Compare December 12, 2024 15:43
achantavy added a commit that referenced this pull request Dec 13, 2024
…tors sync (#1406)

### Summary

Fix for #1404. Made this as a separate PR from
#1405 since I can't
push up to the etsy fork.

### Related issues or links

- #1404


### Checklist

Provide proof that this works (this makes reviews move faster). Please
perform one or more of the following:
- [x] Update/add unit or integration tests.
- [x] Include a screenshot showing what the graph looked like before and
after your changes.
- [ ] Include console log trace showing what happened before and after
your changes.

**Not applicable** If you are changing a node or relationship:
- [ ] Update the
[schema](https://github.com/lyft/cartography/tree/master/docs/root/modules)
and
[readme](https://github.com/lyft/cartography/blob/master/docs/schema/README.md).

**Not applicable** If you are implementing a new intel module:
- [ ] Use the NodeSchema [data
model](https://cartography-cncf.github.io/cartography/dev/writing-intel-modules.html#defining-a-node).

---------

Signed-off-by: Daniel Brauer <[email protected]>
Co-authored-by: Daniel Brauer <[email protected]>
@danbrauer
Copy link
Contributor Author

Closing this PR since its changes got recreated by Alex over in #1406

@danbrauer danbrauer closed this Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant