Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

Match resource owners at top of reduce #1891

Merged
merged 6 commits into from
Jan 5, 2023
Merged

Match resource owners at top of reduce #1891

merged 6 commits into from
Jan 5, 2023

Conversation

ryanfkeepers
Copy link
Contributor

Description

Checks for resource owner matches in the top
of the reduce func using the selector owners,
instead of waiting until the path match check.

Does this PR need a docs update or release note?

  • ⛔ No

Type of change

  • 🌻 Feature

Issue(s)

Test Plan

  • ⚡ Unit test
  • 💚 E2E

@ryanfkeepers ryanfkeepers added the enhancement New feature or request label Dec 21, 2022
@ryanfkeepers ryanfkeepers requested a review from ashmrtn December 21, 2022 00:11
@ryanfkeepers ryanfkeepers self-assigned this Dec 21, 2022
@ryanfkeepers ryanfkeepers temporarily deployed to Testing December 21, 2022 00:11 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing December 21, 2022 00:11 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers changed the title Issue 1617 4 reduceownercompare Match resource owners at top of reduce Dec 21, 2022
@ryanfkeepers ryanfkeepers temporarily deployed to Testing December 21, 2022 00:11 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing December 21, 2022 00:12 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers added the blocked Upstream item prevents completion label Dec 21, 2022
@ryanfkeepers ryanfkeepers force-pushed the issue-1617-3-useselectorowners branch 3 times, most recently from 53e16d1 to 84a3e3f Compare January 4, 2023 00:30
@ryanfkeepers ryanfkeepers force-pushed the issue-1617-4-reduceownercompare branch from a55099f to a891230 Compare January 4, 2023 01:08
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 01:08 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 01:08 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 01:09 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 01:09 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 01:09 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers force-pushed the issue-1617-3-useselectorowners branch 2 times, most recently from e2a5120 to 8df2c30 Compare January 4, 2023 16:15
Base automatically changed from issue-1617-3-useselectorowners to main January 4, 2023 19:39
@ryanfkeepers
match resource owners at top of reduce
8bea389 
Checks for resource owner matches in the top
of the reduce func using the selector owners,
instead of waiting until the path match check.
@ryanfkeepers
prioritize discreteOwners, update changelog
9b48414
@ryanfkeepers ryanfkeepers force-pushed the issue-1617-4-reduceownercompare branch from a891230 to 9b48414 Compare January 4, 2023 20:43
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 20:43 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 20:43 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 20:43 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 21:37 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 21:37 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 21:37 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 21:38 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 21:38 — with GitHub Actions Inactive
ashmrtn
ashmrtn reviewed Jan 4, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
src/cli/utils/exchange.go
}

sel.Include(sel.Users(opts.Users))
sel.Include(sel.Users(users))
Copy link
Contributor

Choose a reason for hiding this comment

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

is this still required? We're already setting users on line 137

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removal will happen in one of the next PRs.

src/cli/utils/exchange.go Show resolved Hide resolved
src/internal/connector/graph_connector_helper_test.go Show resolved Hide resolved
@ashmrtn
Copy link
Contributor

ashmrtn commented Jan 4, 2023

looks like this could use some manual end-to-end testing as well. Doing backup create exchange --user "*" results in errors

We should also make a follow-up issue for how selectors are displayed in the output at the end if multiple users are backed up

corso backup create exchange --user [email protected],[email protected]

  Started At            ID   Status                Selectors                            
  2023-01-04T21:52:56Z  aaa  Completed (0 errors)  [email protected] (1 more)
  2023-01-04T21:53:43Z  bbb  Completed (0 errors)  [email protected] (1 more)

@ryanfkeepers
Copy link
Contributor Author

looks like this could use some manual end-to-end testing as well. Doing backup create exchange --user "*" results in errors

We should also make a follow-up issue for how selectors are displayed in the output at the end if multiple users are backed up

@ashmrtn That bug exists on main. I'll address it directly. And the list display can be kicked to a follow-up PR as well.

@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 22:49 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 22:49 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 22:49 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 22:50 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 22:50 — with GitHub Actions Inactive
@ryanfkeepers ryanfkeepers temporarily deployed to Testing January 4, 2023 22:50 — with GitHub Actions Inactive
@aviator-app
Copy link
Contributor

aviator-app bot commented Jan 5, 2023 •
edited
Loading

Aviator status

Aviator will automatically update this comment as the status of the PR changes.

This PR was merged using Aviator.

@aviator-app aviator-app bot temporarily deployed to Testing January 5, 2023 00:38 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing January 5, 2023 00:38 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing January 5, 2023 00:38 Inactive
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@aviator-app aviator-app bot temporarily deployed to Testing January 5, 2023 00:39 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing January 5, 2023 00:39 Inactive
@aviator-app aviator-app bot temporarily deployed to Testing January 5, 2023 00:39 Inactive
@aviator-app aviator-app bot merged commit 1f26339 into main Jan 5, 2023
@aviator-app aviator-app bot deleted the issue-1617-4-reduceownercompare branch January 5, 2023 00:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ashmrtn ashmrtn ashmrtn approved these changes

Assignees

@ryanfkeepers ryanfkeepers

Labels
enhancement New feature or request mergequeue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ryanfkeepers @ashmrtn