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.

Sign up for GitHub

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

Jump to bottom

drakrun: improve draksetup log output #450

Merged
merged 10 commits into from
Mar 1, 2021
Merged

drakrun: improve draksetup log output #450

merged 10 commits into from
Mar 1, 2021

Conversation

manorit2001
Copy link
Contributor

This PR contains minor improvements which can help users during the installation process

@manorit2001
increased verbosity and user messages
2d17244 
Signed-off-by: manorit chawdhry <manorit2001@gmail.com>
@manorit2001
Copy link
Contributor Author

@chivay which linter should we use for auto-formatting the files?

@manorit2001
Copy link
Contributor Author

Also, is there any specific reason for choosing 4096 as the block size during the calculation of the hash?

@manorit2001
made a function for check_root
0882a4f
@manorit2001
changing the chunk size
ec0cf12 
- the download speed comes to be in kbs which can be increased by
  increasing the chunk size
icedevml
icedevml suggested changes Feb 28, 2021
View reviewed changes
drakrun/drakrun/draksetup.py Outdated Show resolved Hide resolved
@icedevml
Copy link
Contributor

Also, is there any specific reason for choosing 4096 as the block size during the calculation of the hash?

I think it was pretty arbitrary, can be changed to some more feasible value if it really speeds up the calculation.

@icedevml
Copy link
Contributor

which linter should we use for auto-formatting the files?

You can check that in the CI logs:
https://drone.icedev.pl/CERT-Polska/drakvuf-sandbox/1643/2/2

cd drakrun && flake8 --ignore E402,F401,E501

@icedevml
Copy link
Contributor

This PR looks OK to me in general 👍 . Please rename the title to something more feasible, e.g. drakrun: improve draksetup log output, fix linter errors and I think it can be merged then.

manorit2001 and others added 2 commits February 28, 2021 20:27
@manorit2001 @icedevml
Update the required change
c5664e7 
Co-authored-by: Michał Leszczyński <ml@icedev.pl>
@manorit2001
changed random to secrets
16954cb 
- secrets library is cryptographically more better for randomness
@manorit2001
Copy link
Contributor Author

I think it was pretty arbitrary, can be changed to some more feasible value if it really speeds up the calculation.

I haven't really benchmarked anything but just looking at the memory footprint of draksetup, It looked like we can utilize ram more so just tried to increase the value

@manorit2001 manorit2001 changed the title Minor Improvements drakrun: improve draksetup log output Feb 28, 2021
@manorit2001 manorit2001 marked this pull request as ready for review February 28, 2021 15:40
icedevml
icedevml approved these changes Feb 28, 2021
View reviewed changes
Copy link
Contributor

@icedevml icedevml left a comment

Choose a reason for hiding this comment

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

LGTM

chivay
chivay requested changes Mar 1, 2021
View reviewed changes
drakrun/drakrun/drakpdb.py Outdated Show resolved Hide resolved
drakrun/drakrun/draksetup.py Outdated Show resolved Hide resolved
drakrun/drakrun/draksetup.py Outdated Show resolved Hide resolved
drakrun/drakrun/draksetup.py Show resolved Hide resolved
@manorit2001 manorit2001 requested a review from chivay March 1, 2021 16:53
chivay
chivay reviewed Mar 1, 2021
View reviewed changes
drakrun/drakrun/draksetup.py Outdated Show resolved Hide resolved
@chivay chivay merged commit 8e92a04 into CERT-Polska:master Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@icedevml icedevml icedevml approved these changes

@chivay chivay chivay approved these changes

@BonusPlay BonusPlay Awaiting requested review from BonusPlay

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@manorit2001 @icedevml @chivay