Summary: | [ews-app] Add method to save BuilderMapping to database | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Aakash Jain <aakash_jain> | ||||
Component: | Tools / Tests | Assignee: | Aakash Jain <aakash_jain> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | aakash_jain, ap, ews-watchlist, jbedard, kocsen_chung, lforschler, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | Other | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Aakash Jain
2019-02-04 18:15:02 PST
Created attachment 361145 [details]
Proposed patch
Attachment 361145 [details] did not pass style-queue:
ERROR: Tools/BuildSlaveSupport/ews-app/ews/models/buildermapping.py:52: [BuilderMapping.save_mapping] Instance of 'BuilderMapping' has no 'save' member [pylint/E1101] [5]
ERROR: Tools/BuildSlaveSupport/ews-app/ews/models/buildermapping.py:75: [BuilderMapping.get_existing_mapping] Class 'BuilderMapping' has no 'objects' member [pylint/E1101] [5]
Total errors found: 2 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 361145 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=361145&action=review > Tools/BuildSlaveSupport/ews-app/ews/models/buildermapping.py:59 > + _log.error('builder_id {} does not match with builer_id {}. Ignoring new data.'.format(mapping.builder_id, builder_id)) nit: typo: builer Comment on attachment 361145 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=361145&action=review > Tools/BuildSlaveSupport/ews-app/ews/models/buildermapping.py:80 > + def is_valid_mapping(cls, builder_id, builder_name, display_name): Why are we accepting builder_name and display_name if we don't do anything with them? Committed r240977: <https://trac.webkit.org/changeset/240977> > Why are we accepting builder_name and display_name if we don't do anything with them?
It's not required, but I added it for future, we might want to add more validation later on (e.g.: ensuring that display_name is string).
|