Feature/idrs switch by theokaufman · Pull Request #131 · edanalytics/runway

theokaufman · 2026-09-15T22:05:46Z

WIP

Adds in:

IDRS client to wrap API calls
New logic in execution flow to run fuzzy matching in certain circumstances
Executor IDRS orchestration method

Still to come in this or subsequent PRs:

Send logic to return matches to the app
stu-assess-ledger and unsent artifacts

amazon-inspector-ohio · 2026-09-15T22:05:49Z

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

snyk-io-us · 2026-09-15T22:05:57Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

amazon-inspector-ohio · 2026-09-15T22:06:20Z

✅ I finished the code review, and didn't find any security or code quality issues.

theokaufman · 2026-09-15T22:06:31Z

@edandylytics tagged you as reviewer for visibility mainly, but lmk if you have any thoughts!

edandylytics

I haven't tried running locally yet, but thinking tomorrow I'll combine this branch and the one I've got in #128 and see how it shakes out.

edandylytics · 2026-09-16T20:17:53Z

            self.error_url = job["appUrls"]["error"]
            self.matches_url = job["appUrls"]["unmatchedIds"]
            self.summary_url = job["appUrls"]["summary"]
+            self.idrs_url = job["appUrls"]["identityService"]


Note that the app will only send this URL for 'fuzzy' and 'id_based_fuzzy_background' modes -- i.e. when there's an IDRS connection configured and it's ready to be used.

Also fixed this up thank you for noting

edandylytics · 2026-09-16T21:10:46Z

+            if self.id_matching_mode in ['fuzzy', 'id_based_fuzzy_background']:
+                if self.em_runtime and self.em_runtime <= config.MAX_EM_RUNTIME_SECONDS:
+                    self.match_candidates()
+                    self.get_id_matches(artifact.CANDIDATES.path)


Why move the fuzzy matching steps to before the id-based processing has finished? Interleaving the processing modes makes me nervous.

I'm not saying it doesn't work, but I'd want to make very sure:

a new fuzzy matching step failing doesn't break existing id-based matching

the app gets a coherent set of updates from the Executor (e.g. only one earthmover_deps event)

state management between the two processing modes is maintainable long term, since we'll need id-based and fuzzy matching for the foreseeable future (until IDRS becomes a hard requirement to use Runway).

For example, it looks like match_candidates moves the output directory. Will that interfere with where Lightbeam expects the files from the id-based mode to be?

I think we should chat about this a bit more tomorrow to make sure I'm understanding. I think some of this might come down to the naming in the executor at the moment.

As written in this PR, the execution flow is:

id_based -> Run our normal process (encapsulated in orchestrate_earthmover)

fuzzy -> Run our new process (right now spread out over a few methods but the piece that implements earthmover is match_candidates)

Right now, the processes will be separate in the code as written -- The edge case is id_based_fuzzy_background, which we might just want to make another condition in the finally block.

My thought here was that we want to make the "switch" explicit in the execution flow -- run one process OR the other, and afterwards proceed with the rest of the run as normal. I agree with all your concerns, I certainly want to make sure these processes are independent but can coexist within the execution flow. Let me know if I'm misunderstanding anything here!

edandylytics · 2026-09-16T21:14:55Z

+            self.idrs_url = job["appUrls"]["identityService"]
            self.output_files_url = job["appUrls"]["outputFiles"]

+            self.id_matching_mode = job.get("idMatchingMode", True)


id_based as the default?

Fixed up thanks for the note

theokaufman · 2026-09-16T22:40:27Z

I haven't tried running locally yet, but thinking tomorrow I'll combine this branch and the one I've got in #128 and see how it shakes out.

@edandylytics Sounds great please ping me and let me know! I anticipate a decent amount of stuff will need to be wired up, but my feeling is the structure is there to hit the IDRS and handle matches

jayckaiser

I've left some questions, but no major concerns. Andy's callout about chunking the posting to IDRS will be an important improvement to make!

theokaufman · 2026-09-29T20:04:43Z

@jayckaiser Ready for another look when you get a chance!

edandylytics · 2026-09-30T19:55:18Z

+        # The URL for the IDRS
+        self.identity_url = identity_url
+
+    def validate_candidates(self, candidates: list[dict]) -> None:


The IDRS will reject requests with duplicate correlation IDs, and the candidate package doesn't do any deduping. Somewhere, we'll need to weed out duplicates (though I'm not sure this is the place since it'd be mutating the candidates rather than validating them per se...)

Might be good to grab some time tomorrow morning to talk about this validation/mutation distinction (is this a bigger chunk of work than this PR, I wonder?)

Sounds good. FWIW, the validation/mutation distinction was sort of offhand.

But one more thing this function (or something) should do: the candidates package adds a record_count key that'll need to be stripped. The IDRS rejects unexpected keys in order to avoid situations where a caller sends data thinking that it is included in the query and it's not actually factored in.

jayckaiser

@theokaufman I've highlighted a couple more fixes that I missed during our one-on-one this week. Let me know if you have any questions!

jayckaiser · 2026-09-30T21:50:26Z

+
+                resp = self.conn.post(
+                    url=idrs_conn_info['url'],
+                    headers={"Authorization": f"Bearer {idrs_conn_info['token']}"},


Do we need to worry about auth-expiry? Will we ever need to refresh this token mid-run?

I don't think so. We are getting the token in the payload of get_candidates, so it will always be fresh. Any given run that hits the IDRS will use a new token, so I don't think we will need to refresh

The app ensures there's at least 10 min left before the token expires before handing it to the Executor. But if you do get a 401, you can always request a token again from the app. If the 401 was due to token expiration, you'll get a new one.

theokaufman · 2026-10-05T17:19:49Z

@edandylytics heads up that this is good to go!

theokaufman added 4 commits September 15, 2026 17:58

idrs client module

9a5a76a

add IDRS_MATCHES artifact

2341ce6

add IDRSQueryError class

17a3d08

Add IDRS Client orchestration

ff1c368

theokaufman requested a review from jayckaiser September 15, 2026 22:05

theokaufman requested a review from edandylytics September 15, 2026 22:06

edandylytics reviewed Sep 16, 2026

View reviewed changes

theokaufman added 2 commits September 16, 2026 18:29

Set IDRS config fallbacks

71b91e3

rename idrs orchestration method

6c65442

theokaufman added 2 commits September 17, 2026 13:22

fix import

9cfff3b

move idrs orchestration back to finally

abf9b91

edandylytics mentioned this pull request Sep 17, 2026

App/idrs creds to executor #128

Merged

rm outdated idrs_url state

477c6e4

edandylytics reviewed Sep 18, 2026

View reviewed changes

Comment thread executor/executor/idrs_client.py Outdated

jayckaiser requested changes Sep 28, 2026

View reviewed changes

theokaufman added 7 commits September 28, 2026 17:55

Add batch size config

6c5de18

add batching logic to post to the IDRS

a5b9440

cleanup

06fdbc0

Raise if empty array returned

b232112

Collapse if statements

573e4ff

remove needs_upload

0386ce9

add type hinting

dc2fda1

theokaufman marked this pull request as ready for review September 29, 2026 20:04

edandylytics reviewed Sep 30, 2026

View reviewed changes

jayckaiser requested changes Sep 30, 2026

View reviewed changes

update comment

78d9753

jayckaiser approved these changes Oct 5, 2026

View reviewed changes

theokaufman merged commit d5ca7f8 into development Oct 5, 2026
4 checks passed

theokaufman deleted the feature/idrs_switch branch October 5, 2026 20:25

edandylytics mentioned this pull request Oct 6, 2026

Release/10 6 26 #148

Merged

Conversation

theokaufman commented Sep 15, 2026

Uh oh!

amazon-inspector-ohio Bot commented Sep 15, 2026

Uh oh!

snyk-io-us Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

amazon-inspector-ohio Bot commented Sep 15, 2026

Uh oh!

theokaufman commented Sep 15, 2026

Uh oh!

edandylytics left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theokaufman commented Sep 16, 2026

Uh oh!

Uh oh!

jayckaiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

theokaufman commented Sep 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edandylytics Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jayckaiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theokaufman commented Oct 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

snyk-io-us Bot commented Sep 15, 2026 •

edited

Loading

edandylytics Sep 30, 2026 •

edited

Loading