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.

Sign up for GitHub

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

Jump to bottom

Add workflow to run conda-store user journey tests #2895

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Add workflow to run conda-store user journey tests #2895

wants to merge 14 commits into from

Conversation

soapy1
Copy link
Contributor

@soapy1 soapy1 commented Jan 6, 2025 •
edited
Loading

Reference Issues or PRs

fixes #2760

  • Runs conda-store integration tests as part of test_local_integration
  • Adds the option to specify a --group flag when adding a keycloak user

To create a keycloak user with access to a particular group, run the command

nebari keycloak adduser --user username password --config nebari-config.yaml --groups mygroup

What does this implement/fix?

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe): CI change

@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch 11 times, most recently from 8bb4c2a to 814d7e8 Compare January 7, 2025 22:09
@soapy1
Copy link
Contributor Author

soapy1 commented Jan 8, 2025

depends on conda-incubator/conda-store#1040

@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch from 814d7e8 to b58c5c7 Compare January 8, 2025 19:11
@peytondmurray peytondmurray mentioned this pull request Jan 9, 2025
Merged
3 tasks
@viniciusdc
Copy link
Contributor

Since these tests depend on a live cluster, I suggest extending the already existing local integration tests with this extra check https://github.com/nebari-dev/nebari/blob/main/.github/workflows/test_local_integration.yaml -- since it uses CiRun under the hood you should not be affected by any flakiness of the GH runner in case of constrained resources.

@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch 15 times, most recently from 8d941b5 to c108c2f Compare January 16, 2025 21:52
@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch 2 times, most recently from 550fe91 to 88d000f Compare February 6, 2025 17:57
@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch 2 times, most recently from 8138e62 to b1079d1 Compare February 12, 2025 21:07
@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch 2 times, most recently from 7fbbdf9 to 9652b91 Compare February 13, 2025 16:30
@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch from 9652b91 to a4e24a5 Compare February 13, 2025 16:30
@soapy1 soapy1 marked this pull request as ready for review February 13, 2025 16:30
@soapy1 soapy1 requested a review from a team as a code owner February 13, 2025 16:30
@soapy1 soapy1 requested review from dcmcand and removed request for a team February 13, 2025 16:30
soapy1
soapy1 commented Feb 13, 2025
View reviewed changes
src/_nebari/keycloak.py
@@ -29,7 +29,10 @@ def do_keycloak(config: schema.Main, *args):

username = args[1]
password = args[2] if len(args) >= 3 else None
create_user(keycloak_admin, username, password, domain=config.domain)
groups = args[3] if len(args) >= 4 else None
Copy link
Contributor Author

@soapy1 soapy1 Feb 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not a fan of this approach to passing arguments around but did this to follow the pattern already defined in this part of the code.
I would be happy to refactor this module to make it a more developer friendly if that's something the nebari team is into.
Could also split that into a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

why don't you split that into a different pr. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like this functionality is also implemented in #2917. Will rebase my changes when it get's merged.

@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch from 21578d2 to a17192b Compare February 13, 2025 19:03
@soapy1 soapy1 force-pushed the conda-store-black-box-testing branch from a17192b to 43a603c Compare February 13, 2025 19:04
@dcmcand dcmcand added the status: approved 💪🏾 This PR has been reviewed and approved for merge label Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dcmcand dcmcand dcmcand approved these changes

@aktech aktech Awaiting requested review from aktech

@viniciusdc viniciusdc Awaiting requested review from viniciusdc

Assignees
No one assigned
Labels
status: approved 💪🏾 This PR has been reviewed and approved for merge
Projects
🪴 Nebari Project Management
Status: New 🚦
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ENH] - add conda-store black-box testing to ci
4 participants
@soapy1 @viniciusdc @aktech @dcmcand