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

48-interval-schema #50

Merged
merged 3 commits into from
Jul 15, 2024
Merged

Conversation

rc10house
Copy link
Contributor

Adds a polymorphic type UserAleSchema that is auto-evaluated to either a UserAle raw log or a UserAle interval log upon validation. This change introduces some problems with serialization, namely the order of fields is not preserved anymore compared to the original log. This is due to the specific raw and interval logs inheriting from a base UserAleBase class that contains the shared fields. When serialized, the UserAleBase fields appear first followed by the fields exclusive to the specific log. There appears to be no native way to fix this in Pydantic at the moment. This PR will probably require some discussion and changes.

Closes #48

Copy link
Contributor

@EandrewJones EandrewJones left a comment

Choose a reason for hiding this comment

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

I don't see a good reason to split the UserALE schemas into three separate files. They're closely coupled (developers are likely to need to bounce back and forth between the files when working on the userale schema; therefore keeping in the same file simplifies cognitive load) and it repeats lots of imports.

A few other nits. Almost there.

distill/core/log.py Outdated Show resolved Hide resolved
distill/core/log.py Outdated Show resolved Hide resolved
distill/core/log.py Outdated Show resolved Hide resolved
distill/core/log.py Outdated Show resolved Hide resolved
distill/schemas/useralebase.py Outdated Show resolved Hide resolved
@EandrewJones EandrewJones merged commit 5048552 into apache:master Jul 15, 2024
4 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.

Feature(Schema): Support UserALE Interval Schema
3 participants