Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Fix logging issue from bug #1477 #1478

Merged
merged 1 commit into from
Dec 11, 2019
Merged

Conversation

doanac
Copy link
Collaborator

@doanac doanac commented Dec 9, 2019

This is subtle so I'm keeping what's really two changes as one commit:

The issue is that DockerAppManger's constructor needs to create a
"fake fetcher" that's not really used but required for constructing
member variables and they don't have access to the global Config. Only
the PackageManagerConfig. Calling the Config default constructor
results in a call to postUpdateValues that then calls
logger_set_threshold with the default logging level, INFO.

This commit updates the Fetch constructor to only use the values that
it needs from the globabl config: repo-server and director-server. The
DockerAppMgr constructor then doesn't to create a fake global config.

Signed-off-by: Andy Doan [email protected]

This is subtle so I'm keeping what's really two changes as one commit:

The issue is that DockerAppManger's constructor needs to create a
"fake fetcher" that's not really used but required for constructing
member variables and they don't have access to the global Config. Only
the PackageManagerConfig. Calling the `Config` default constructor
results in a call to `postUpdateValues` that then calls
`logger_set_threshold` with the default logging level, INFO.

This commit updates the Fetch constructor to only use the values that
it needs from the globabl config: repo-server and director-server. The
DockerAppMgr constructor then doesn't to create a fake global config.

Signed-off-by: Andy Doan <[email protected]>
@codecov-io
Copy link

codecov-io commented Dec 9, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@da7d9bb). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1478   +/-   ##
=========================================
  Coverage          ?   80.48%           
=========================================
  Files             ?      184           
  Lines             ?    11092           
  Branches          ?        0           
=========================================
  Hits              ?     8927           
  Misses            ?     2165           
  Partials          ?        0
Impacted Files Coverage Δ
...rc/libaktualizr/package_manager/dockerappmanager.h 80% <100%> (ø)
src/libaktualizr/uptane/fetcher.cc 100% <100%> (ø)
src/libaktualizr/uptane/fetcher.h 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da7d9bb...247de0c. Read the comment docs.

@lbonn lbonn merged commit a699eb9 into advancedtelematic:master Dec 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants