Skip to content

Commit

Permalink
Add CI format checks for the Jupyter web app (kubeflow#5811)
Browse files Browse the repository at this point in the history
* jwa(front): Add npm rule for checking the format

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* ci: Add common tasks for format checks

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* ci(jwa): Add format check tasks

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* jwa(back): Fix formatting

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* jwa(front): Fix formatting

Run `npm run format:write` on frontend

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* jwa(front): Include prettier in package.json

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* jwa(docker): Don't copy node_modules in Dockerfile

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* web-apps: Add global dockerignore file

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* jwa: Remove unused dockerignore file

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>

* jwa(make): Don't include dockerignore and cleanup

Signed-off-by: Kimonas Sotirchos <kimwnasptd@arrikto.com>
  • Loading branch information
kimwnasptd authored Apr 8, 2021
1 parent e8250b9 commit 43e03d4
Show file tree
Hide file tree
Showing 58 changed files with 347 additions and 277 deletions.
2 changes: 2 additions & 0 deletions components/crud-web-apps/.dockerignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
**/node_modules/*
**/__pycache__/*
13 changes: 9 additions & 4 deletions components/crud-web-apps/jupyter/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ FROM node:12-buster-slim as frontend-kubeflow-lib

WORKDIR /src

ENV NG_CLI_ANALYTICS "ci"
COPY ./common/frontend/kubeflow-common-lib/package*.json ./
RUN npm install
RUN npm ci

COPY ./common/frontend/kubeflow-common-lib/ .
RUN npm run build
Expand All @@ -21,11 +22,15 @@ RUN npm run build
FROM node:12-buster-slim as frontend

WORKDIR /src

COPY ./jupyter/frontend/package*.json ./
RUN npm install
COPY --from=frontend-kubeflow-lib /src/dist/kubeflow/ ./node_modules/kubeflow/
COPY ./jupyter/frontend/tsconfig*.json ./
COPY ./jupyter/frontend/angular.json ./
COPY ./jupyter/frontend/src ./src

COPY ./jupyter/frontend/ .
ENV NG_CLI_ANALYTICS "ci"
RUN npm ci
COPY --from=frontend-kubeflow-lib /src/dist/kubeflow/ ./node_modules/kubeflow/

RUN npm run build -- --output-path=./dist/default --configuration=production
RUN npm run build -- --output-path=./dist/rok --configuration=rok-prod
Expand Down
11 changes: 0 additions & 11 deletions components/crud-web-apps/jupyter/Dockerfile.dockerignore

This file was deleted.

55 changes: 3 additions & 52 deletions components/crud-web-apps/jupyter/Makefile
Original file line number Diff line number Diff line change
@@ -1,58 +1,9 @@
# Copyright 2017 The Kubernetes Authors.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

# Project used with GCB
PROJECT ?= kubeflow-dev
# Registry where the image should be published
REGISTRY_PROJECT ?= kubeflow-dev
OUTPUT ?= output.yaml

# We want the git tag to be the last commit to this directory so we don't
# bump the image on unrelated changes.
GIT_TAG ?= $(shell git log -n 1 --pretty=format:"%h" ./)

IMG ?= gcr.io/${REGISTRY_PROJECT}/jupyter-web-app

GIT_TAG ?= $(shell git describe --tags --always)
IMG ?= public.ecr.aws/j1r0q0g6/notebooks/jupyter-web-app
DOCKERFILE ?= jupyter/Dockerfile

info:
echo image: \"gcr.io/$(REGISTRY_PROJECT)/jupyter-web-app:$(GIT_TAG)\" > $(OUTPUT)

# TODO(kimwnasptd): we will need to update the gcb scripts to be able to use a
# different context. The web app should be build from the context:
# components/crud-web-apps in order to have access to both the web app's code
# as well as the common code.
build-gcb: info
gcloud --project=$(PROJECT) \
builds submit \
--machine-type=n1-highcpu-32 \
--substitutions=_GIT_VERSION=$(GIT_TAG),_REGISTRY=$(REGISTRY_PROJECT) \
--config=cloudbuild.yaml .

build-gcb-win: info
cmd /c gcloud --project=$(PROJECT) \
builds submit \
--machine-type=n1-highcpu-32 \
--substitutions=_GIT_VERSION=$(GIT_TAG),_REGISTRY=$(REGISTRY_PROJECT) \
--config=cloudbuild.yaml .

docker-build:
cp Dockerfile.dockerignore ../.dockerignore
-cd ../ && docker build -t ${IMG}:${GIT_TAG} -f ${DOCKERFILE} .
rm ../.dockerignore

cd ../ && docker build -t ${IMG}:${GIT_TAG} -f ${DOCKERFILE} .

docker-push:
docker push $(IMG):$(GIT_TAG)
Expand Down
18 changes: 11 additions & 7 deletions components/crud-web-apps/jupyter/backend/apps/common/form.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@

log = logging.getLogger(__name__)

SERVER_TYPE_ANNOTATION = "notebooks.kubeflow.org/server-type"
HEADERS_ANNOTATION = "notebooks.kubeflow.org/http-headers-request-set"
URI_REWRITE_ANNOTATION = "notebooks.kubeflow.org/http-rewrite-uri"


def get_form_value(body, defaults, body_field, defaults_field=None):
"""
Expand Down Expand Up @@ -139,20 +143,20 @@ def set_server_type(notebook, body, defaults):
valid_server_types = ["jupyter", "vs-code", "rstudio"]
notebook_annotations = notebook["metadata"]["annotations"]
server_type = get_form_value(body, defaults, "serverType")
if server_type == "" :
if server_type == "":
server_type == "jupyter"
if server_type not in valid_server_types:
raise BadRequest("'%s' is not a valid server type" % server_type)
raise BadRequest("'%s' is not a valid server type" % server_type)

nb_name = get_form_value(body, defaults, "name")
nb_ns = get_form_value(body, defaults, "namespace")
rstudio_header = ('{"X-RStudio-Root-Path":"/notebook/%s/%s/"}' % (nb_ns, nb_name))

notebook_annotations["notebooks.kubeflow.org/server-type"] = server_type
rstudio_header = '{"X-RStudio-Root-Path":"/notebook/%s/%s/"}' % (nb_ns,
nb_name)
notebook_annotations[SERVER_TYPE_ANNOTATION] = server_type
if server_type == "vs-code" or server_type == "rstudio":
notebook_annotations["notebooks.kubeflow.org/http-rewrite-uri"] = "/"
notebook_annotations[URI_REWRITE_ANNOTATION] = "/"
if server_type == "rstudio":
notebook_annotations["notebooks.kubeflow.org/http-headers-request-set"] = rstudio_header
notebook_annotations[HEADERS_ANNOTATION] = rstudio_header


def set_notebook_cpu(notebook, body, defaults):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

bp = Blueprint("base_routes", __name__)

from . import get, delete, patch # noqa: F401
from . import delete, get, patch # noqa: F401, E402
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

bp = Blueprint("default_routes", __name__)

from . import post # noqa: F401
from . import post # noqa: F401, E402
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from flask import request

from kubeflow.kubeflow.crud_backend import (api, decorators, helpers,
logging)
from kubeflow.kubeflow.crud_backend import api, decorators, helpers, logging

from ...common import form, utils
from . import bp
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from flask import request

from kubeflow.kubeflow.crud_backend import (api, decorators, helpers,
logging)
from kubeflow.kubeflow.crud_backend import api, decorators, helpers, logging

from ...common import form, utils
from ..utils import common as rok_common
Expand Down
2 changes: 1 addition & 1 deletion components/crud-web-apps/jupyter/backend/entrypoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def get_config(mode):
cfg_class = config_classes.get(mode)
if not cfg_class:
raise RuntimeError("Backend mode '%s' is not implemented. Choose one"
" of %s" % (mode, list(config_classes.keys())))
" of %s" % (mode, list(config_classes.keys())))
return cfg_class()


Expand Down
6 changes: 6 additions & 0 deletions components/crud-web-apps/jupyter/frontend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion components/crud-web-apps/jupyter/frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
"build:watch": "npm run copyLibAssets && ng build --watch --deploy-url static/ --outputPath ../backend/apps/default/static/ --outputHashing all",
"build:watch:rok": "npm run copyLibAssets && ng build --watch --deploy-url static/ --outputPath ../backend/apps/rok/static/ --outputHashing all --configuration=rok",
"copyLibAssets": "cp ./node_modules/kubeflow/assets/* ./src/assets/",
"format:check": "prettier --check 'src/**/*.{js,ts,html,scss,css}' || node scripts/check-format-error.js",
"format:write": "prettier --write 'src/**/*.{js,ts,html,scss,css}'",
"test": "ng test",
"lint": "ng lint",
"e2e": "ng e2e"
Expand Down Expand Up @@ -45,9 +47,9 @@
"@angular/cli": "~8.3.20",
"@angular/compiler-cli": "~8.2.14",
"@angular/language-service": "~8.2.14",
"@types/node": "^12.11.1",
"@types/jasmine": "~3.5.0",
"@types/jasminewd2": "~2.0.3",
"@types/node": "^12.11.1",
"codelyzer": "^5.1.2",
"jasmine-core": "~3.5.0",
"jasmine-spec-reporter": "~4.2.1",
Expand All @@ -56,6 +58,7 @@
"karma-coverage-istanbul-reporter": "~2.1.0",
"karma-jasmine": "~2.0.1",
"karma-jasmine-html-reporter": "^1.4.2",
"prettier": "^2.2.1",
"protractor": "~5.4.3",
"ts-node": "~8.3.0",
"tslint": "~5.18.0",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
console.error();
console.error('-----------------------------------------------');
console.error('Please run `npm run format:write` to format your code.');
console.error('-----------------------------------------------');
console.error();
process.exit(1);
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { FormComponent } from './pages/form/form.component';

const routes: Routes = [
{ path: '', component: IndexComponent },
{ path: 'new', component: FormComponent},
{ path: 'new', component: FormComponent },
];

@NgModule({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,8 @@ import { AppComponent } from './app.component';
describe('AppComponent', () => {
beforeEach(async(() => {
TestBed.configureTestingModule({
imports: [
RouterTestingModule
],
declarations: [
AppComponent
],
imports: [RouterTestingModule],
declarations: [AppComponent],
}).compileComponents();
}));

Expand All @@ -30,6 +26,8 @@ describe('AppComponent', () => {
const fixture = TestBed.createComponent(AppComponent);
fixture.detectChanges();
const compiled = fixture.nativeElement;
expect(compiled.querySelector('.content span').textContent).toContain('frontend app is running!');
expect(compiled.querySelector('.content span').textContent).toContain(
'frontend app is running!',
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Component } from '@angular/core';
@Component({
selector: 'app-root',
templateUrl: './app.component.html',
styleUrls: ['./app.component.scss']
styleUrls: ['./app.component.scss'],
})
export class AppComponent {
title = 'frontend';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { async, ComponentFixture, TestBed } from "@angular/core/testing";
import { async, ComponentFixture, TestBed } from '@angular/core/testing';

import { FormAdvancedOptionsComponent } from "./form-advanced-options.component";
import { FormAdvancedOptionsComponent } from './form-advanced-options.component';

describe("FormAdvancedOptionsComponent", () => {
describe('FormAdvancedOptionsComponent', () => {
let component: FormAdvancedOptionsComponent;
let fixture: ComponentFixture<FormAdvancedOptionsComponent>;

beforeEach(async(() => {
TestBed.configureTestingModule({
declarations: [FormAdvancedOptionsComponent]
declarations: [FormAdvancedOptionsComponent],
}).compileComponents();
}));

Expand All @@ -18,7 +18,7 @@ describe("FormAdvancedOptionsComponent", () => {
fixture.detectChanges();
});

it("should create", () => {
it('should create', () => {
expect(component).toBeTruthy();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ describe('FormAffinityTolerationsComponent', () => {

beforeEach(async(() => {
TestBed.configureTestingModule({
declarations: [ FormAffinityTolerationsComponent ]
})
.compileComponents();
declarations: [FormAffinityTolerationsComponent],
}).compileComponents();
}));

beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@ import { TolerationGroup, AffinityConfig } from 'src/app/types';
@Component({
selector: 'app-form-affinity-tolerations',
templateUrl: './form-affinity-tolerations.component.html',
styleUrls: ['./form-affinity-tolerations.component.scss']
styleUrls: ['./form-affinity-tolerations.component.scss'],
})
export class FormAffinityTolerationsComponent implements OnInit {
@Input() parentForm: FormGroup;
@Input() tolerationGroups: TolerationGroup[];
@Input() affinityConfigs: AffinityConfig[];

constructor() { }

ngOnInit() {
}
constructor() {}

ngOnInit() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ describe('FormConfigurationsComponent', () => {

beforeEach(async(() => {
TestBed.configureTestingModule({
declarations: [ FormConfigurationsComponent ]
})
.compileComponents();
declarations: [FormConfigurationsComponent],
}).compileComponents();
}));

beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@ describe('FormCpuRamComponent', () => {

beforeEach(async(() => {
TestBed.configureTestingModule({
declarations: [ FormCpuRamComponent ]
})
.compileComponents();
declarations: [FormCpuRamComponent],
}).compileComponents();
}));

beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { async, ComponentFixture, TestBed } from "@angular/core/testing";
import { async, ComponentFixture, TestBed } from '@angular/core/testing';

import { FormDataVolumesComponent } from "./form-data-volumes.component";
import { FormDataVolumesComponent } from './form-data-volumes.component';

describe("FormDataVolumesComponent", () => {
describe('FormDataVolumesComponent', () => {
let component: FormDataVolumesComponent;
let fixture: ComponentFixture<FormDataVolumesComponent>;

beforeEach(async(() => {
TestBed.configureTestingModule({
declarations: [FormDataVolumesComponent]
declarations: [FormDataVolumesComponent],
}).compileComponents();
}));

Expand All @@ -18,7 +18,7 @@ describe("FormDataVolumesComponent", () => {
fixture.detectChanges();
});

it("should create", () => {
it('should create', () => {
expect(component).toBeTruthy();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@
LAUNCH
</button>

<button mat-raised-button type="button" (click)="onCancel()">
CANCEL
</button>
<button mat-raised-button type="button" (click)="onCancel()">CANCEL</button>
</div>
</div>
Loading

0 comments on commit 43e03d4

Please sign in to comment.