]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
mgr/dashboard: Cleanup 24831/head
authorPatrick Nawracay <pnawracay@suse.com>
Mon, 29 Oct 2018 08:59:59 +0000 (09:59 +0100)
committerPatrick Nawracay <pnawracay@suse.com>
Thu, 15 Nov 2018 09:23:02 +0000 (10:23 +0100)
- Make a method static instead of keeping it as classmethod
- Fix return type of methods type hint
- Reformat to fit 100 chars max width
- Rename some variables, methods and argument names for a better
  understanding

  Examples:
    - activatedCompression() -> hasCompressionEnabled()
    - extendByItemsForSubmit() -> assignFormFields()
        - api -> fieldName
        - name -> formControlName
        - fn -> replaceFn
        - edit -> editable
- Extract type declaration to interface and adapted the code to use
  that interface
- Replace unused nearly-global variables with local ones
- Add types where missing and appropriate
  Exmaples:
    - PoolService::getInfo() -> Observable<PoolFormInfo>
- Moved model declaration to shared directory
- Added documentation for a method which isn't that easy to understand

Signed-off-by: Patrick Nawracay <pnawracay@suse.com>
src/pybind/mgr/dashboard/controllers/pool.py
src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-info.ts [deleted file]
src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.html
src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.spec.ts
src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form.component.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/api/erasure-code-profile.service.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/api/pool.service.ts
src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts [new file with mode: 0644]

index 7f0254c6d814240e83d2f7ec6c80a67d360840af..1964204d88ef96c34779a9b4ef7b4a602695929e 100644 (file)
@@ -19,8 +19,8 @@ def pool_task(name, metadata, wait_for=2.0):
 @ApiController('/pool', Scope.POOL)
 class Pool(RESTController):
 
-    @classmethod
-    def _serialize_pool(cls, pool, attrs):
+    @staticmethod
+    def _serialize_pool(pool, attrs):
         if not attrs or not isinstance(attrs, list):
             attrs = pool.keys()
 
@@ -65,9 +65,8 @@ class Pool(RESTController):
             raise cherrypy.NotFound('No such pool')
         return pool[0]
 
-    # '_get' will be wrapped into JSON through '_request_wrapper'
     def get(self, pool_name, attrs=None, stats=False):
-        # type: (str, str, bool) -> str
+        # type: (str, str, bool) -> dict
         return self._get(pool_name, attrs, stats)
 
     @pool_task('delete', ['{pool_name}'])
@@ -145,10 +144,10 @@ class Pool(RESTController):
                        for o in mgr.get('osd_metadata').values())
 
         def compression_enum(conf_name):
-            return [[v for v in o['enum_values'] if len(v) > 0] for o in config_options
+            return [[v for v in o['enum_values'] if len(v) > 0]
+                    for o in mgr.get('config_options')['options']
                     if o['name'] == conf_name][0]
 
-        config_options = mgr.get('config_options')['options']
         return {
             "pool_names": [p['pool_name'] for p in self._pool_list()],
             "crush_rules_replicated": rules(1),
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-info.ts b/src/pybind/mgr/dashboard/frontend/src/app/ceph/pool/pool-form/pool-form-info.ts
deleted file mode 100644 (file)
index e1e56ea..0000000
+++ /dev/null
@@ -1,12 +0,0 @@
-import { CrushRule } from '../../../shared/models/crush-rule';
-
-export class PoolFormInfo {
-  pool_names: string[];
-  osd_count: number;
-  is_all_bluestore: boolean;
-  bluestore_compression_algorithm: string;
-  compression_algorithms: string[];
-  compression_modes: string[];
-  crush_rules_replicated: CrushRule[];
-  crush_rules_erasure: CrushRule[];
-}
index 4d99d8e84c15e279dee6b3403d30f098b6e6df7f..96f9b8bb20684f8b6c51252830976716b434bf7a 100644 (file)
                 </select>
               </div>
             </div>
-            <div *ngIf="activatedCompression()">
+            <div *ngIf="hasCompressionEnabled()">
               <!-- Compression algorithm selection -->
               <div class="form-group"
                    [ngClass]="{'has-error': form.showError('algorithm', formDir)}">
index 5770d240f5ea817f9db772e7b32b62c7a9a0f8e8..da5351aec17cbcafe129d891a282eb924e940652 100644 (file)
@@ -216,7 +216,7 @@ describe('PoolFormComponent', () => {
       ['crushRule', 'size', 'erasureProfile', 'ecOverwrites'].forEach((name) =>
         formHelper.expectValid(name)
       );
-      expect(component.compressionForm.valid).toBeTruthy();
+      expect(component.form.get('compression').valid).toBeTruthy();
     });
 
     it('validates name', () => {
@@ -297,7 +297,7 @@ describe('PoolFormComponent', () => {
       });
 
       it('is valid', () => {
-        expect(component.compressionForm.valid).toBeTruthy();
+        expect(component.form.get('compression').valid).toBeTruthy();
       });
 
       it('validates minBlobSize to be only valid between 0 and maxBlobSize', () => {
index a43198ff222eafdedbf4e3faf413a6079d5beb2c..56ea0e5875f98f6579debba21d3c739bb7a0dbf2 100644 (file)
@@ -16,6 +16,7 @@ import { CrushStep } from '../../../shared/models/crush-step';
 import { ErasureCodeProfile } from '../../../shared/models/erasure-code-profile';
 import { FinishedTask } from '../../../shared/models/finished-task';
 import { Permission } from '../../../shared/models/permissions';
+import { PoolFormInfo } from '../../../shared/models/pool-form-info';
 import { DimlessBinaryPipe } from '../../../shared/pipes/dimless-binary.pipe';
 import { AuthStorageService } from '../../../shared/services/auth-storage.service';
 import { FormatterService } from '../../../shared/services/formatter.service';
@@ -23,7 +24,15 @@ import { TaskWrapperService } from '../../../shared/services/task-wrapper.servic
 import { ErasureCodeProfileFormComponent } from '../erasure-code-profile-form/erasure-code-profile-form.component';
 import { Pool } from '../pool';
 import { PoolFormData } from './pool-form-data';
-import { PoolFormInfo } from './pool-form-info';
+
+interface FormFieldDescription {
+  externalFieldName: string;
+  formControlName: string;
+  attr?: string;
+  replaceFn?: Function;
+  editable?: boolean;
+  resetValue?: any;
+}
 
 @Component({
   selector: 'cd-pool-form',
@@ -33,7 +42,6 @@ import { PoolFormInfo } from './pool-form-info';
 export class PoolFormComponent implements OnInit {
   permission: Permission;
   form: CdFormGroup;
-  compressionForm: CdFormGroup;
   ecProfiles: ErasureCodeProfile[];
   info: PoolFormInfo;
   routeParamsSubscribe: any;
@@ -59,7 +67,7 @@ export class PoolFormComponent implements OnInit {
   ) {
     this.editing = this.router.url.startsWith('/pool/edit');
     this.authenticate();
-    this.createForms();
+    this.createForm();
   }
 
   authenticate() {
@@ -72,8 +80,8 @@ export class PoolFormComponent implements OnInit {
     }
   }
 
-  private createForms() {
-    this.compressionForm = new CdFormGroup({
+  private createForm() {
+    const compressionForm = new CdFormGroup({
       mode: new FormControl('none'),
       algorithm: new FormControl(''),
       minBlobSize: new FormControl('', {
@@ -86,6 +94,7 @@ export class PoolFormComponent implements OnInit {
         updateOn: 'blur'
       })
     });
+
     this.form = new CdFormGroup(
       {
         name: new FormControl('', {
@@ -117,7 +126,7 @@ export class PoolFormComponent implements OnInit {
           validators: [Validators.required, Validators.min(1)]
         }),
         ecOverwrites: new FormControl(false),
-        compression: this.compressionForm
+        compression: compressionForm
       },
       CdValidators.custom('form', () => null)
     );
@@ -389,19 +398,19 @@ export class PoolFormComponent implements OnInit {
   }
 
   private setCompressionValidators() {
-    CdValidators.validateIf(this.form.get('minBlobSize'), () => this.activatedCompression(), [
+    CdValidators.validateIf(this.form.get('minBlobSize'), () => this.hasCompressionEnabled(), [
       Validators.min(0),
       CdValidators.custom('maximum', (size) =>
         this.oddBlobSize(size, this.form.getValue('maxBlobSize'))
       )
     ]);
-    CdValidators.validateIf(this.form.get('maxBlobSize'), () => this.activatedCompression(), [
+    CdValidators.validateIf(this.form.get('maxBlobSize'), () => this.hasCompressionEnabled(), [
       Validators.min(0),
       CdValidators.custom('minimum', (size) =>
         this.oddBlobSize(this.form.getValue('minBlobSize'), size)
       )
     ]);
-    CdValidators.validateIf(this.form.get('ratio'), () => this.activatedCompression(), [
+    CdValidators.validateIf(this.form.get('ratio'), () => this.hasCompressionEnabled(), [
       Validators.min(0),
       Validators.max(1)
     ]);
@@ -413,7 +422,7 @@ export class PoolFormComponent implements OnInit {
     return Boolean(minimum && maximum && minimum >= maximum);
   }
 
-  activatedCompression() {
+  hasCompressionEnabled() {
     return this.form.getValue('mode') && this.form.get('mode').value.toLowerCase() !== 'none';
   }
 
@@ -468,53 +477,70 @@ export class PoolFormComponent implements OnInit {
       this.form.setErrors({ cdSubmitButton: true });
       return;
     }
+
     const pool = { pool: this.form.getValue('name') };
-    this.extendByItemsForSubmit(pool, [
-      { api: 'pool_type', name: 'poolType' },
-      { api: 'pg_num', name: 'pgNum', edit: true },
+
+    this.assignFormFields(pool, [
+      { externalFieldName: 'pool_type', formControlName: 'poolType' },
+      { externalFieldName: 'pg_num', formControlName: 'pgNum', editable: true },
       this.form.getValue('poolType') === 'replicated'
-        ? { api: 'size', name: 'size' }
-        : { api: 'erasure_code_profile', name: 'erasureProfile', attr: 'name' },
-      { api: 'rule_name', name: 'crushRule', attr: 'rule_name' }
+        ? { externalFieldName: 'size', formControlName: 'size' }
+        : {
+            externalFieldName: 'erasure_code_profile',
+            formControlName: 'erasureProfile',
+            attr: 'name'
+          },
+      { externalFieldName: 'rule_name', formControlName: 'crushRule', attr: 'rule_name' }
     ]);
+
     if (this.info.is_all_bluestore) {
-      this.extendByItemForSubmit(pool, {
-        api: 'flags',
-        name: 'ecOverwrites',
-        fn: () => ['ec_overwrites']
+      this.assignFormField(pool, {
+        externalFieldName: 'flags',
+        formControlName: 'ecOverwrites',
+        replaceFn: () => ['ec_overwrites']
       });
+
       if (this.form.getValue('mode') !== 'none') {
-        this.extendByItemsForSubmit(pool, [
+        this.assignFormFields(pool, [
           {
-            api: 'compression_mode',
-            name: 'mode',
-            edit: true,
-            fn: (value) => this.activatedCompression() && value
+            externalFieldName: 'compression_mode',
+            formControlName: 'mode',
+            editable: true,
+            replaceFn: (value) => this.hasCompressionEnabled() && value
           },
-          { api: 'compression_algorithm', name: 'algorithm', edit: true },
           {
-            api: 'compression_min_blob_size',
-            name: 'minBlobSize',
-            fn: this.formatter.toBytes,
-            edit: true,
+            externalFieldName: 'compression_algorithm',
+            formControlName: 'algorithm',
+            editable: true
+          },
+          {
+            externalFieldName: 'compression_min_blob_size',
+            formControlName: 'minBlobSize',
+            replaceFn: this.formatter.toBytes,
+            editable: true,
             resetValue: 0
           },
           {
-            api: 'compression_max_blob_size',
-            name: 'maxBlobSize',
-            fn: this.formatter.toBytes,
-            edit: true,
+            externalFieldName: 'compression_max_blob_size',
+            formControlName: 'maxBlobSize',
+            replaceFn: this.formatter.toBytes,
+            editable: true,
             resetValue: 0
           },
-          { api: 'compression_required_ratio', name: 'ratio', edit: true, resetValue: 0 }
+          {
+            externalFieldName: 'compression_required_ratio',
+            formControlName: 'ratio',
+            editable: true,
+            resetValue: 0
+          }
         ]);
       } else if (this.editing) {
-        this.extendByItemsForSubmit(pool, [
+        this.assignFormFields(pool, [
           {
-            api: 'compression_mode',
-            name: 'mode',
-            edit: true,
-            fn: () => 'unset'
+            externalFieldName: 'compression_mode',
+            formControlName: 'mode',
+            editable: true,
+            replaceFn: () => 'unset'
           }
         ]);
       }
@@ -526,41 +552,44 @@ export class PoolFormComponent implements OnInit {
     this.triggerApiTask(pool);
   }
 
-  private extendByItemsForSubmit(pool, items: any[]) {
-    items.forEach((item) => this.extendByItemForSubmit(pool, item));
+  /**
+   * Retrieves the values for the given form field descriptions and assigns the values to the given
+   * object. This method differentiates between `add` and `edit` mode and acts differently on one or
+   * the other.
+   */
+  private assignFormFields(pool: object, formFieldDescription: FormFieldDescription[]): void {
+    formFieldDescription.forEach((item) => this.assignFormField(pool, item));
   }
 
-  private extendByItemForSubmit(
-    pool,
+  /**
+   * Retrieves the value for the given form field description and assigns the values to the given
+   * object. This method differentiates between `add` and `edit` mode and acts differently on one or
+   * the other.
+   */
+  private assignFormField(
+    pool: object,
     {
-      api,
-      name,
+      externalFieldName,
+      formControlName,
       attr,
-      fn,
-      edit,
+      replaceFn,
+      editable,
       resetValue
-    }: {
-      api: string;
-      name: string;
-      attr?: string;
-      fn?: Function;
-      edit?: boolean;
-      resetValue?: any;
-    }
-  ) {
-    if (this.editing && (!edit || this.form.get(name).pristine)) {
+    }: FormFieldDescription
+  ): void {
+    if (this.editing && (!editable || this.form.get(formControlName).pristine)) {
       return;
     }
-    const value = this.form.getValue(name);
-    let apiValue = fn ? fn(value) : attr ? _.get(value, attr) : value;
+    const value = this.form.getValue(formControlName);
+    let apiValue = replaceFn ? replaceFn(value) : attr ? _.get(value, attr) : value;
     if (!value || !apiValue) {
-      if (edit && !_.isUndefined(resetValue)) {
+      if (editable && !_.isUndefined(resetValue)) {
         apiValue = resetValue;
       } else {
         return;
       }
     }
-    pool[api] = apiValue;
+    pool[externalFieldName] = apiValue;
   }
 
   private triggerApiTask(pool) {
index 165a5d6a562afdb785c0335df8f3f0bfb5937147..7599caaa60d6177a1dd30d6fe02b73eb033f5f6c 100644 (file)
@@ -1,6 +1,8 @@
 import { HttpClient } from '@angular/common/http';
 import { Injectable } from '@angular/core';
 
+import { Observable } from 'rxjs';
+
 import { ErasureCodeProfile } from '../models/erasure-code-profile';
 import { ApiModule } from './api.module';
 
@@ -12,8 +14,8 @@ export class ErasureCodeProfileService {
 
   constructor(private http: HttpClient) {}
 
-  list() {
-    return this.http.get(this.apiPath);
+  list(): Observable<ErasureCodeProfile[]> {
+    return this.http.get<ErasureCodeProfile[]>(this.apiPath);
   }
 
   create(ecp: ErasureCodeProfile) {
index 183b1c4081d8829c347cc249a648ffd91f11ba7e..59f4eec686769f14dc61a4d8b2f660881bce3561 100644 (file)
@@ -1,7 +1,10 @@
 import { HttpClient } from '@angular/common/http';
 import { Injectable } from '@angular/core';
 
+import { Observable } from 'rxjs';
+
 import { cdEncode } from '../decorators/cd-encode';
+import { PoolFormInfo } from '../models/pool-form-info';
 import { ApiModule } from './api.module';
 
 @cdEncode
@@ -35,8 +38,8 @@ export class PoolService {
     return this.http.get(this.apiPath);
   }
 
-  getInfo() {
-    return this.http.get(`${this.apiPath}/_info`);
+  getInfo(): Observable<PoolFormInfo> {
+    return this.http.get<PoolFormInfo>(`${this.apiPath}/_info`);
   }
 
   list(attrs = []) {
diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/models/pool-form-info.ts
new file mode 100644 (file)
index 0000000..7741087
--- /dev/null
@@ -0,0 +1,12 @@
+import { CrushRule } from './crush-rule';
+
+export class PoolFormInfo {
+  pool_names: string[];
+  osd_count: number;
+  is_all_bluestore: boolean;
+  bluestore_compression_algorithm: string;
+  compression_algorithms: string[];
+  compression_modes: string[];
+  crush_rules_replicated: CrushRule[];
+  crush_rules_erasure: CrushRule[];
+}