-
Notifications
You must be signed in to change notification settings - Fork 88
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
Explicit config [WIP] #2294
Open
Adam-D-Lewis
wants to merge
139
commits into
develop
Choose a base branch
from
explicit_config
base: develop
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Explicit config [WIP] #2294
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
10 tasks
Opened an RFD to address new requirements - nebari-dev/governance#48 |
Adam-D-Lewis
commented
May 6, 2024
"PTH", | ||
"E", # E: pycodestyle rules | ||
"F", # F: pyflakes rules | ||
"PTH", # PTH: flake8-use-pathlib rules | ||
] | ||
ignore = [ | ||
"E501", # Line too long | ||
"F821", # Undefined name | ||
"PTH123", # open() should be replaced by Path.open() | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes a deprecation warning from ruff.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reference Issues or PRs
Fixes #2122, based on #2348 which should be merged first
Currently, we set all the nebari-config fields manually in
nebari/src/_nebari/initialize.py
Line 33 in 0210a47
but it's confusing for users to not see some useful parts of the config (e.g. #2122). We could add more fields to render_config method, but every time we add a new field in any InputSchema, we'd have to update the render_config method which is inconvenient. Additionally, having the fields hard coded in render_config means Nebari extensions don't have their InputSchema's written to the config file even if installed. They would need to be added some other way (e.g. typing them in manually) in order to control the extension's settings exposed by it's InputSchema.
This PR instead writes all of the InputSchema's to the nebari-config file by default, but this led to some confusing config in cases like infrastructure InputSchema where this PR caused a section to be added for each of "local", "existing", "google_cloud_platform", etc. for each cloud prodiver and all except one of these provider sections was blank.
nebari/src/_nebari/stages/infrastructure/__init__.py
Lines 540 to 546 in 0210a47
So this PR also allows InputSchemas to define a
exclude_from_config
method which returns the names of the fields to exclude from the config file to fix this issue. We likely could address this issue by separating the different providers into separated Pydantic Models (similar to what I've done to the certificate Pydantic models), but I feel it's important to have some mechanism to manually exclude portions of the InputSchema from being written to the nebari-config file when runningnebari init
, and have to chosen to address the extra providers withexclude_from_config
.What does this implement/fix?
Put a
x
in the boxes that applynebari init
results in a more complete nebari-config.yamlTesting
Any other comments?