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 checker to check value type of configuration properties #2298

Open
toan-quach opened this issue Dec 3, 2024 · 1 comment · May be fixed by #2328
Open

Add checker to check value type of configuration properties #2298

toan-quach opened this issue Dec 3, 2024 · 1 comment · May be fixed by #2328
Assignees
@joaoandre-avaiga
Labels
⚙️Configuration Related to Taipy Configuration Core: ⚙️ Configuration Core Related to Taipy Core 📄 Documentation Internal or public documentation 📈 Improvement Improvement of a feature. 🟧 Priority: High Must be addressed as soon 📝Release Notes Impacts the Release Notes or the Documentation in general

Comments

@toan-quach
Copy link
Member

toan-quach commented Dec 3, 2024 •
edited
Loading

  1. We should also add all the properties to check. not only the callable.

  2. The dictionary should be moved close to the definition of the properties, in the DataNodeCOnfig class.

  3. The format should be updated to Dict[str, Dict[str, Any]] from Dict[str, List[Tuple[Any, List[str]]]]

Example:

{
    DataNodeConfig._STORAGE_TYPE_VALUE_GENERIC: 
        {
            DataNodeConfig._OPTIONAL_READ_FUNCTION_GENERIC_PROPERTY: Callable,
            DataNodeConfig._OPTIONAL_WRITE_FUNCTION_GENERIC_PROPERTY: Callable
        },
    DataNodeConfig._STORAGE_TYPE_VALUE_SQL:
        {
            DataNodeConfig._REQUIRED_WRITE_QUERY_BUILDER_SQL_PROPERTY: Callable,
            DataNodeConfig._OPTIONAL_APPEND_QUERY_BUILDER_SQL_PROPERTY: Callable
        }
}
  1. Add the changes to documentation
@toan-quach toan-quach added Core Related to Taipy Core 📈 Improvement Improvement of a feature. 📄 Documentation Internal or public documentation ⚙️Configuration Related to Taipy Configuration 🟧 Priority: High Must be addressed as soon 📝Release Notes Impacts the Release Notes or the Documentation in general Core: ⚙️ Configuration labels Dec 3, 2024
@joaoandre-avaiga joaoandre-avaiga self-assigned this Dec 11, 2024
@toan-quach
Copy link
Member Author

Hi @joaoandre-avaiga, @jrobinAV

I want to ask you guys if we can expand this idea a bit further. I want to add 2 points that will make this ticket more complicated 😅

  1. Can we also include parameters that can be None?
  2. Can we somehow declare that in case the value is None, we want to throw a warning except for an error (the message might have to be declared elsewhere)

I'm thinking about this as I found a missing implementation in the current databricks ticket: https://github.com/Avaiga/taipy-enterprise/pull/557/files#diff-6dd483af9d43a6ce79f513e4d9cd499ddceef45cddcbb1a30b589aace7255b30R81

@joaoandre-avaiga joaoandre-avaiga linked a pull request Dec 13, 2024 that will close this issue
Draft
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@joaoandre-avaiga joaoandre-avaiga

Labels
⚙️Configuration Related to Taipy Configuration Core: ⚙️ Configuration Core Related to Taipy Core 📄 Documentation Internal or public documentation 📈 Improvement Improvement of a feature. 🟧 Priority: High Must be addressed as soon 📝Release Notes Impacts the Release Notes or the Documentation in general
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add data config checker for property value types Avaiga/taipy
2 participants
@joaoandre-avaiga @toan-quach