Skip to content

Add prefix to secret key#878

Merged
vinnamkim merged 1 commit into
open-edge-platform:releases/1.1.0from
vinnamkim:bugfix/add-prefix-to-secret-key
Mar 22, 2023
Merged

Add prefix to secret key#878
vinnamkim merged 1 commit into
open-edge-platform:releases/1.1.0from
vinnamkim:bugfix/add-prefix-to-secret-key

Conversation

@vinnamkim
Copy link
Copy Markdown
Contributor

@vinnamkim vinnamkim commented Mar 22, 2023

Summary

  • From @chuneuny-emily's stress test, we observed that the DatumaroBinary format encryption integration test occasionally fails because argparse expects one argument for --encryption-key.
self = ArgumentParser(prog='datumaro_binary', usage=None, description=None, formatter_class=<class 'datumaro.cli.util.MultilineFormatter'>, conflict_handler='error', add_help=True)
status = 2, message = 'datumaro_binary: error: argument --encryption-key: expected one argument\n'
  • This is because the secret key can randomly start with - and argparse recognizes it as a new argument directive.
self = ArgumentParser(prog='datumaro_binary', usage=None, description=None, formatter_class=<class 'datumaro.cli.util.MultilineFormatter'>, conflict_handler='error', add_help=True)
arg_strings = ['--encryption-key', '-VaiTYwWN41UinEp6S5ke1zPnCh3hoZajOCEVptpyt8='], namespace = Namespace(encryption_key=None)

# '-VaiTYwWN41UinEp6S5ke1zPnCh3hoZajOCEVptpyt8=' starts with '-'.
  • This PR resolves this issue by adding datum- prefix to the secret key.
  • Remove a redundant file also.

How to test

The existing tests cover this change.

Checklist

  • I have added unit tests to cover my changes.​
  • I have added integration tests to cover my changes.​
  • I have added the description of my changes into CHANGELOG.​
  • I have updated the documentation accordingly

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2023 Intel Corporation
#
# SPDX-License-Identifier: MIT

 - Remove redundant file also

Signed-off-by: Kim, Vinnam <vinnam.kim@intel.com>
@vinnamkim vinnamkim marked this pull request as ready for review March 22, 2023 02:33
@vinnamkim vinnamkim added the BUG label Mar 22, 2023
@vinnamkim vinnamkim added this to the 1.1.0 milestone Mar 22, 2023
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 22, 2023

Codecov Report

Patch coverage: 81.25% and project coverage change: +0.05 🎉

Comparison is base (fc7f607) 78.49% compared to head (ca1f6b5) 78.55%.

Additional details and impacted files
@@                Coverage Diff                 @@
##           releases/1.1.0     #878      +/-   ##
==================================================
+ Coverage           78.49%   78.55%   +0.05%     
==================================================
  Files                 206      205       -1     
  Lines               24939    24943       +4     
  Branches             5007     5009       +2     
==================================================
+ Hits                19577    19595      +18     
+ Misses               4236     4222      -14     
  Partials             1126     1126              
Flag Coverage Δ
macos-11_Python-3.8 77.54% <81.25%> (+0.05%) ⬆️
ubuntu-20.04_Python-3.8 78.55% <81.25%> (+0.05%) ⬆️
windows-2019_Python-3.8 78.49% <81.25%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
datumaro/components/crypter.py 80.00% <81.25%> (-0.86%) ⬇️

... and 10 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@chuneuny-emily chuneuny-emily added FIX and removed BUG labels Mar 22, 2023
Comment thread datumaro/components/crypter.py
Comment thread datumaro/components/crypter.py
@vinnamkim vinnamkim merged commit 1f8e45e into open-edge-platform:releases/1.1.0 Mar 22, 2023
@vinnamkim vinnamkim deleted the bugfix/add-prefix-to-secret-key branch March 22, 2023 06:00
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.

4 participants