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

Unmark sandbox_version as readonly #683

Merged
merged 4 commits into from
Jan 31, 2023

Conversation

hmd34
Copy link
Contributor

@hmd34 hmd34 commented Oct 26, 2022

🔧 Changes

Previously, there was a issue about sandbox_version Import. #69
So mark sandbox_version to readonly, and remove sandbox_version in export.

Now seems to have been resolved that issue. Fix so that sandbox_version is exported again.

📚 References

Fix: #682

🔬 Testing

Following steps

  1. Check the Node.js runtime version in tenant advanced settings
  2. Export settings with a0deploy
  3. Check to appears sandbox_version in tenant.json
  4. Compare 1 and 3 step's Node version

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

Refs: auth0#682

Previously, there was a issue about sandbox_version Import a trouble.
auth0#69
So mark sandbox_version to readonly, and remove sandbox_version in export.

Now seems to have been resolved that issue.
Fix so that sandbox_version is exported again.
@hmd34 hmd34 requested a review from a team as a code owner October 26, 2022 13:31
@hmd34 hmd34 marked this pull request as draft October 26, 2022 13:32
@hmd34 hmd34 marked this pull request as ready for review October 26, 2022 13:50
@hmd34
Copy link
Contributor Author

hmd34 commented Oct 26, 2022

E2E tests failed. I saw errors and don't know why failed. Do I need to take any action?

@Widcket
Copy link
Contributor

Widcket commented Nov 7, 2022

cc @willvedd

@willvedd
Copy link
Contributor

@hmd34 I haven't forgotten about this. Can confirm that support for updating sandbox_version was added sometime between #69 and now. I want to be particularly careful to not introduce any regressions for folks so need to double-check that including this property in payloads is compatible with all environments and permutations of the API.

Copy link
Contributor

@willvedd willvedd left a comment

Choose a reason for hiding this comment

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

@hmd34 thanks for your patience here. I needed to confirm that this property was configurable and can confirm. I went ahead and updated the test recordings to get this ready for approval.

@willvedd
Copy link
Contributor

Despite the CircleCI checks suggesting otherwise, all tests are indeed passing. My suspicion is that this fork's lack of environment variables being injected is interfering with the recordings. Going ahead with a merge anyway, we'll have to fix that integration later.

@willvedd willvedd merged commit 8413c55 into auth0:master Jan 31, 2023
@hmd34
Copy link
Contributor Author

hmd34 commented Feb 12, 2023

@willvedd So sorry for delayed response and thank you for your work.

@hmd34 hmd34 deleted the to-be-export-sandbox-version branch February 12, 2023 04:27
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.

Sandbox_version is imported but cannot be exported
3 participants