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

Fix some attributes and state for KC export #4370

Merged
merged 1 commit into from
Dec 18, 2024
Merged

Conversation

rhysyngsun
Copy link
Contributor

What are the relevant tickets?

N/A

What's this PR do?

  • Switches createdTimestamp to an integer in milliseconds. The previous stringified value was being parse typically as some datetime in January of 1970. No clear reason why, no documentation as to what it should be aside from a long. API didn't seem to reject the value though, although arbitrary string values did get rejected. Timestamps in seconds had the same result.
  • Added a --verbose flag that logs the requests, error responses are still always logged
  • Added handling for skipped users, with a new action field on the model to reflect whether a user was actually exported or skipped because they already exist. Without this users who already existed were attempted to export perpetually.

How should this be manually tested?

Run ./manage.py migrate_users_to_keycloak --client-id admin-cli --client-secret SECRET --verbose and you should see verbose output.

Copy link

sentry-io bot commented Dec 18, 2024

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: keycloak_user_export/management/commands/migrate_users_to_keycloak.py

Function Unhandled Issue
handle JSONDecodeError: Expecting value: line 1 column 1 (char 0) keycloak_user_export.management.commands.migrate_users_to_keycloak in _re...
Event Count: 1

Did you find this useful? React with a 👍 or 👎

@rhysyngsun rhysyngsun marked this pull request as ready for review December 18, 2024 19:18
@rhysyngsun rhysyngsun requested a review from a team as a code owner December 18, 2024 19:18
@rhysyngsun rhysyngsun merged commit 4c3d802 into master Dec 18, 2024
6 checks passed
@rhysyngsun rhysyngsun deleted the nl/fix-errors branch December 18, 2024 19:42
@odlbot odlbot mentioned this pull request Dec 18, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants