Skip to content

DAOS-4860: Update Jenkinsfile with bandit.config - #2797

Merged
jolivier23 merged 5 commits into
masterfrom
DAOS-4860_1
Jun 1, 2020
Merged

jolivier23 merged 5 commits into
masterfrom
DAOS-4860_1

Conversation

@dinghwah

@dinghwah dinghwah commented May 27, 2020 •

Copy link
Copy Markdown
Contributor

DAOS-4860 test: To update Jenkinsfile to add bandit.config when bandit run.

Test-tag: pr,hw
Signed-off-by: Ding Ho ding-hwa.ho@intel.com

modified: Jenkinsfile

Test-tag: pr,hw,large
Signed-off-by: Ding Ho <ding-hwa.ho@intel.com>
@daosbuild1

Copy link
Copy Markdown
Collaborator

jolivier23
jolivier23 previously approved these changes May 28, 2020
@dinghwah
dinghwah requested review from dpquigl and kjacque May 28, 2020 17:07

@brianjmurrell brianjmurrell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't you change the default disabled state of the bandit stage to default-enabled but skipable?

daos/Jenkinsfile

Lines 374 to 375 in 698d03b

cachedCommitPragma(pragma: 'Skip-python-bandit',
def_val: 'true') != 'true'

such as:

expression { ! skip_stage('checkpatch') }

Also, not sure why you added Test-tag: pr,hw,large to your commit message. Adding the large tag for the VM tests doesn't make any sense. What was your goal there?

dinghwah added 2 commits May 28, 2020 18:45
Update Jenkinsfile to enable bandit.check by default

Test-tag: pr,hw
Signed-off-by: Ding Ho <ding-hwa.ho@intel.com>
@daosbuild1

Copy link
Copy Markdown
Collaborator

Test stage Python Bandit check completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-2797/2/execution/node/70/log

kjacque
kjacque previously approved these changes May 28, 2020
@dinghwah

Copy link
Copy Markdown
Contributor Author

@brian, Thank you for the review and comment.
I think we need to skip the bandit.check by default, since it check all the python codes for every new PR. On commit #3, I enable the bandit.check I am also seeing another 30+ bandit errors for the recent checked-in scripts, we need to take care these issues separately.

dinghwah added 2 commits May 28, 2020 20:43
modified: Jenkinsfile to enable skip-bandit-check by default

Test-tag: pr,hw
Signed-off-by: Ding Ho <ding-hwa.ho@intel.com>
@daosbuild1

Copy link
Copy Markdown
Collaborator

@brianjmurrell

Copy link
Copy Markdown
Contributor

I am also seeing another 30+ bandit errors for the recent checked-in scripts, we need to take care these issues separately.

Is there a ticket for that?

TBH, I'm not sure I see the point in landing this if we continue to have bandit disabled. IOW, why not fix the remaining issues, and then land this, enabling bandit?

@dinghwah

Copy link
Copy Markdown
Contributor Author

@brian The file bandit.config was missing in the beginning, which should be merged.
Which is not related to enable skip bandit.check turned by default.
The new bandit issue has been tracking by DAOS-5044,

@dinghwah
dinghwah requested review from johannlombardi and mjmac June 1, 2020 18:09
@jolivier23
jolivier23 merged commit d0fc361 into master Jun 1, 2020
@jolivier23
jolivier23 deleted the DAOS-4860_1 branch June 1, 2020 19:29
SABollinger pushed a commit that referenced this pull request Nov 11, 2020
Update bandit to use config file

Signed-off-by: Ding Ho <ding-hwa.ho@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants