Skip to content

Conversation

jubrad
Copy link
Collaborator

@jubrad jubrad commented Jun 23, 2023

#10

Adds retries via reqwest_retry / reqwest_middlware.

Caution!!!
I'm bad at rust

@jubrad jubrad requested review from benesch and evanharmon June 23, 2023 19:25
@CLAassistant
Copy link

CLAassistant commented Jun 23, 2023

CLA assistant check
All committers have signed the CLA.

Comment on lines +159 to +160
assert_eq!(user.name, name);
assert_eq!(user.email, email);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We definitely want to make sure the user name is the name we set during create here, rather than the CreateUser object they give us, which is unfortunately wrong at the moment.

@jubrad
Copy link
Collaborator Author

jubrad commented Jun 23, 2023

The tests work locally when I use a new frontegg workspace.

I the tests are failing here because I don't have access to the secret because I'm using a fork. I could add secrets to my forked repo, but the workflow file client_id is hard coded. If I were to parameterize that in the code the CI tests might work here, but they'd break for non-forked PRs since there probably isn't a secret.frontegg_client_id set for this repo.

@evanharmon
Copy link
Contributor

The tests work locally when I use a new frontegg workspace.

I the tests are failing here because I don't have access to the secret because I'm using a fork. I could add secrets to my forked repo, but the workflow file client_id is hard coded. If I were to parameterize that in the code the CI tests might work here, but they'd break for non-forked PRs since there probably isn't a secret.frontegg_client_id set for this repo.

My first PR was a fork as well. If I remember correctly, I ended up closing the initial PR with the fork. Then I did a new PR from within the repo (pushed to origin) and it was able to pass CI / be merged.

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.

3 participants