Skip to content

Updated Marketplace models & log_level settings#14

Merged
SteveMcGrath merged 1 commit intomainfrom
feature/updated_logging_and_mp_models
Feb 13, 2025
Merged

Updated Marketplace models & log_level settings#14
SteveMcGrath merged 1 commit intomainfrom
feature/updated_logging_and_mp_models

Conversation

@SteveMcGrath
Copy link
Contributor

Description

  • Updated the marketplace & pyproject models to support resource minimums
  • Updated the marketplace & pyproject models to support timeout min/max settings (BREAKING)
  • Updated the Settings model and added implicit log_level setting with a default of "INFO"
  • Removed settings import from JSON/TOML file as they are unused and unneeded.
  • Refactored the run command to improve readability
  • Refactored logging initialization to cleanly handle falling through missing log levels.
  • Fixed identified issue where the json_data, while logged, would never be output as it was passed before log handler assignment

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Unit testing and additional tests have been added

Test Configuration:

  • Python Version(s) Tested: 3.12

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@SteveMcGrath SteveMcGrath force-pushed the feature/updated_logging_and_mp_models branch from 67719c6 to ce22091 Compare February 12, 2025 23:25
@SteveMcGrath SteveMcGrath marked this pull request as ready for review February 12, 2025 23:25
@SteveMcGrath SteveMcGrath requested a review from a team as a code owner February 12, 2025 23:25

[tool.ruff.format]
line-ending = "lf"
#quote-style="single"
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove comment or uncomment

) -> BaseModel:
def fetch_config(self, data: str) -> (BaseModel, int):
"""
Fetch and validate the configuration from either the data string or the filepath
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove file path reference in comment

elif config:
lvl = config.log_level
else:
lvl = "DEBUG"
Copy link
Contributor

Choose a reason for hiding this comment

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

The model default is INFO. Not a big deal, just surfacing the delta. Also, might as well use your enum

Comment on lines +3 to +4
from pydantic import BaseModel, Field, model_validator
from pydantic.functional_validators import model_validator
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont see where model_validator is used, but it's somewhere hidden from me, alias one of these. I suspect this is IDE (un)helpfully importing a dependency and not cleaning up.

Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding SemVer as project.version type

]


class TenintConnectorTimeout(BaseModel):
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment with unit (seconds)



class TenintConnectorResources(BaseModel):
disk: int = 1024
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment with unit (MB)

# The default maximum run time in seconds
timeout = 3600
[tool.tenint.connector.timeout]
default = 3600
Copy link
Contributor

Choose a reason for hiding this comment

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

Does validate_default=True do some magic that populates timeout.min and timeout.max?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup. forces the default value to enter sub-model

Comment on lines +51 to +58
# No longer need to test file input as this functionality has been removed as it's
# no longer necessary at this time.
#
# def test_connector_fetch_config_json_file(AppSettings, tmp_path):
# fn = tmp_path.joinpath("example.json")
# fn.write_text('{"is_bool": true}')
# connector = Connector(settings=AppSettings)
# assert connector.fetch_config(fn=fn) == AppSettings(is_bool=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete comment

Comment on lines +61 to +65
# def test_connector_fetch_config_toml_file(AppSettings, tmp_path):
# fn = tmp_path.joinpath("example.toml")
# fn.write_text("is_bool = true")
# connector = Connector(settings=AppSettings)
# assert connector.fetch_config(fn=fn) == AppSettings(is_bool=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete

@SteveMcGrath SteveMcGrath merged commit b270d53 into main Feb 13, 2025
3 checks passed
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.

2 participants