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

Explicit config [WIP] #2294

Open
wants to merge 139 commits into
base: develop
Choose a base branch
from
Open

Explicit config [WIP] #2294

wants to merge 139 commits into from

Conversation

Adam-D-Lewis
Copy link
Member

@Adam-D-Lewis Adam-D-Lewis commented Mar 6, 2024

Reference Issues or PRs

Fixes #2122, based on #2348 which should be merged first

Currently, we set all the nebari-config fields manually in

def render_config(

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.

class InputSchema(schema.Base):
local: Optional[LocalProvider]
existing: Optional[ExistingProvider]
google_cloud_platform: Optional[GoogleCloudPlatformProvider]
amazon_web_services: Optional[AmazonWebServicesProvider]
azure: Optional[AzureProvider]
digital_ocean: Optional[DigitalOceanProvider]

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 running nebari init, and have to chosen to address the extra providers with exclude_from_config.

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): nebari init results in a more complete nebari-config.yaml

Testing

  • Did you test the pull request locally?
  • Did you add new tests?

Any other comments?

@Adam-D-Lewis
Copy link
Member Author

Opened an RFD to address new requirements - nebari-dev/governance#48

"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()
]
Copy link
Member Author

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
Labels
status: in progress 🏗 This task is currently being worked on
Projects
Status: In progress 🏗
Development

Successfully merging this pull request may close these issues.

[BUG] - Config generated by guided init should contain default settings
5 participants