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

Exclude RequestStatus from the returned values of get_modifiable_properties #12108

Conversation

todor-ivanov
Copy link
Contributor

@todor-ivanov todor-ivanov commented Sep 24, 2024

Fixes #12037

Status

Ready

Description

with the current change we remove the RequestStatus key from the returned values of get_modifiable_properties. this should avoid the conflict created between the set of values checked during the validation and the set of values supported in the WEB interface (the number of visualized keys in the web interface).

Is it backward compatible (if not, which system it affects?)

YES

Related PRs

A Fix of a bug eventually introduced with: #12077

External dependencies / deployment changes

No

@todor-ivanov
Copy link
Contributor Author

todor-ivanov commented Sep 24, 2024

@amaltaro I think this may solve the probelm, but I still experience some issues with my dev cluster (communicated in the cmsweb channel), so I am not able to test it yet.

@cmsdmwmbot
Copy link

Jenkins results:

  • Python3 Unit tests: failed
    • 1 new failures
  • Python3 Pylint check: failed
    • 1 warnings and errors that must be fixed
    • 7 warnings
    • 53 comments to review
  • Pylint py3k check: succeeded
  • Pycodestyle check: succeeded
    • 7 comments to review

Details at https://cmssdt.cern.ch/dmwm-jenkins/view/All/job/DMWM-WMCore-PR-test/15227/artifact/artifacts/PullRequestReport.html

@todor-ivanov
Copy link
Contributor Author

Hi @amaltaro I just tested the fix - it's doing the job properly. we can safely merge this one

Copy link
Contributor

@amaltaro amaltaro left a comment

Choose a reason for hiding this comment

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

Thanks Todor

@amaltaro amaltaro merged commit d80165b into dmwm:master Sep 25, 2024
3 of 4 checks passed
@amaltaro
Copy link
Contributor

@todor-ivanov it looks like we missed the the failing unit test:

    WMCore_t.ReqMgr_t.Utils_t.Validation_t.ValidationTests:testValidateRequestAllowedArgs changed from success to failure

Please provide a new PR with the relevant fix (fixing the original issue 12107).

@todor-ivanov
Copy link
Contributor Author

todor-ivanov commented Sep 25, 2024

This whole comment here was wrongly posted to the current PR. Now deleted and moved to: #12111 (comment)

@todor-ivanov
Copy link
Contributor Author

todor-ivanov commented Sep 25, 2024

This whole comment here was wrongly posted to the current PR. Now deleted and moved to: #12111 (comment)

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.

Support change to SiteWhitelist/SiteBlacklist in ReqMgr2 for active workflows
3 participants