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

Make logging consistent from Cosmos operators #1157

Open
tatiana opened this issue Aug 14, 2024 · 0 comments
Open

Make logging consistent from Cosmos operators #1157

tatiana opened this issue Aug 14, 2024 · 0 comments
Labels
area:logging Related to logging, like log levels, log formats, error logging, etc

Comments

@tatiana
Copy link
Collaborator

tatiana commented Aug 14, 2024

While implementing #1079, we ended up using logger.info as opposed to self.log.info in a few parts of the code to overcome a testing issue where caplog was not able to capture logs that were generated using self.log.info.

This ticket aims to make logging consistent from Cosmos operators. It may be solved by #1108

@dosubot dosubot bot added the area:logging Related to logging, like log levels, log formats, error logging, etc label Aug 14, 2024
tatiana added a commit that referenced this issue Aug 16, 2024
## Description

Added `virtualenv_dir` as an option to `ExecutionConfig` which is then
propagated downstream to `DbtVirtualenvBaseOperator`.

The following now happens:
- If the flag is set, the operator will attempt to locate the `venv`'s
`python` binary under the provided `virtualenv_dir`.
- If so, it will conclude that the `venv` exists and continues without
creating a new one.
    - If not, it will create a new one at `virtualenv_dir`
- If the flag is not set, simply continue using the temporary directory
solution that was already in place.

## Impact
A very basic test using a local `docker compose` set-up as per the
contribution guide and the
[example_virtualenv](https:/astronomer/astronomer-cosmos/blob/main/dev/dags/example_virtualenv.py)
DAG saw the DAG's runtime go down from **2m31s** to just **32s**. I'd
this improvement to be even more noticeable with more complex graphs and
more python requirements.
 
## Related Issue(s)
Closes: #610 
Partially solves: #1042
Follow up ticket: #1157

## Breaking Change?
None, the flag is optional and is ignored (with a
[warning](https:/astronomer/astronomer-cosmos/compare/main...LennartKloppenburg:astronomer-cosmos:feature/cache-virtualenv?expand=1#diff-61b585fb903927b6868b9626c95e0ec47e3818eb477d795ebd13b0276d4fd76cR125))
when used outside of `VirtualEnv` execution mode.

## Important notice

Most of the changes in this PR were originally implemented in PR #611 by
@LennartKloppenburg. It became stale over the last few months due to
limited maintainer availability. Our sincere apologies to the original
author.

What was accomplished since:
1. Rebased
2. Fixed conflicts
3. Fixed failing tests
4. Introduced new tests

Co-authored-by: Lennart Kloppenburg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:logging Related to logging, like log levels, log formats, error logging, etc
Projects
None yet
Development

No branches or pull requests

1 participant