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

Validate final type after URL eval #1217

Merged
merged 8 commits into from Jul 14, 2023
Merged

Validate final type after URL eval #1217

merged 8 commits into from Jul 14, 2023

Conversation

jmosbacher
Copy link
Member

If type is given, validate the config type after URL evaluation.
Also added the run_id to the cache key.

straxen/url_config.py Show resolved Hide resolved
straxen/url_config.py Outdated Show resolved Hide resolved
straxen/url_config.py Show resolved Hide resolved
straxen/url_config.py Outdated Show resolved Hide resolved
straxen/url_config.py Outdated Show resolved Hide resolved
straxen/url_config.py Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Jul 9, 2023

Coverage Status

coverage: 93.556% (+0.01%) from 93.543% when pulling dbf6991 on validate_url_final_type into cb5527a on master.

@jmosbacher jmosbacher requested a review from dachengx July 9, 2023 18:42
@dachengx
Copy link
Collaborator

Thanks @jmosbacher . Are codes that can change the results only def validate_type?

@jmosbacher
Copy link
Member Author

@dachengx Oh right, sorry about that. I ran auto formatting so there are a bunch of formatting changes not really relevant to the actual change.
The validate_type and fetch methods indeed include the actual "logic" changes.
I am a bit on the fence whether we should warn when the type is incorrect or raise an error. Any opinion?

@dachengx
Copy link
Collaborator

No problem. Since type does not influence lineage, maybe we can raise errors and also modify the current code if needed.

@jmosbacher
Copy link
Member Author

Ok, I will switch it to raise an error.

Copy link
Collaborator

@dachengx dachengx left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks @jmosbacher .

@dachengx dachengx merged commit 03ab8b8 into master Jul 14, 2023
7 checks passed
@dachengx dachengx deleted the validate_url_final_type branch July 14, 2023 12:55
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.

None yet

3 participants