-
Notifications
You must be signed in to change notification settings - Fork 5
[unstable] Add dry run option #457
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
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #457 +/- ##
==========================================
- Coverage 78.98% 78.49% -0.49%
==========================================
Files 42 42
Lines 3650 3716 +66
==========================================
+ Hits 2883 2917 +34
- Misses 767 799 +32
🚀 New features to boost your workflow:
|
| def post(self, url: str, json: dict, **kwargs: dict[str, Any]) -> _MockResponse: | ||
| self._logger.info(f"[DRY-RUN] SKIPPED POST to {url} with payload: {json}") | ||
| return self._MockResponse(url) | ||
|
|
||
| def get(self, url: str, **kwargs: dict[str, Any]) -> _MockResponse: | ||
| self._logger.info(f"[DRY-RUN] SKIPPED GET from {url}") | ||
| return self._MockResponse(url) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all good, but I'd also like for us to be able to report what we're sending to the cdf. I believe the idea of a dry run especially with a combination of both dry run and debug, is for the user to know what the extractor is doing at every step.
If possible, we should also be able to show what the expected response for each data sent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed the mock response accordingly to the application config being fetched from api or not.
Logging the response properly.
| self._extractor_class.CONFIG_TYPE, | ||
| ) | ||
|
|
||
| return application_config, current_config_revision |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should probably try to use pyright as your lsp because this is very unsafe. You could result in a situation where you get UnboundLocalError since resolving application_config and current_config_revision is behind a condition. What then happens where connection_config is None?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this to fix UnboundLocalError
|
I just realized that for this to be a complete dry run, I'm not sure how this could be done, but we also need to make sure that no data flows from all the tasks. This may require that we also create a dry run of sorts for the various uploaders we have. |
This is a simple CLI option that allows users to run the extractor in dry-run mode. Users will be able to use -dry-run flag.
Note:
This PR is currently blocked by another change.