Adopt shared SDK and reconcile ambiguous uploads - #2
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1459f3d194
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| token_url = os.getenv(f"{env_prefix}_TOKEN_URL", "") | ||
| if not client_id or not client_secret or not token_url: | ||
| company_id = os.getenv(f"{env_prefix}_COMPANY_ID", os.getenv("SF_COMPANY_ID", "")) | ||
| if not client_id or not client_secret or not token_url or not company_id: |
There was a problem hiding this comment.
Wire OAuth company ID before requiring it
With this new check, OAuth clients built from the web UI cannot be created: the OAuth sections in web_ui/templates/index.html only collect client id/secret/token URL, and _test_one_connection/process only export those SF_* env vars before calling build_sf_client. Because company_id is only read from SF_<env>_COMPANY_ID/SF_COMPANY_ID, selecting OAuth in the UI fails with this AuthError unless a server-side env var happens to be pre-set; either add/wire the company-id field for OAuth or pass it from the submitted config before this validation.
Useful? React with 👍 / 👎.
3e5f148 to
bb8a004
Compare
What changed
sapsf-sharedclient and authentication contractAmbiguousWriteErrorfor create operationsexternalCodeinstead of replaying POSTWhy
A SuccessFactors create can commit successfully while its response is lost. Replaying that POST can produce a duplicate or a misleading failure. The uploader now checks target state and safely continues dependent hierarchy uploads when the original object exists.
Validation
pytest: 104 passedgit diff --check: passedMerge order
Merge SahirVhora/sapsf-shared#2 first. This PR imports
AmbiguousWriteErrorintroduced by that upstream change.Note: the existing OAuth company-ID review comment predates this safety fix and remains separate follow-up work.