You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by "ocket8888 (via GitHub)" <gi...@apache.org> on 2023/03/01 22:06:46 UTC
[GitHub] [trafficcontrol] ocket8888 opened a new pull request, #7375: Basic ISO generation page/form
ocket8888 opened a new pull request, #7375:
URL: https://github.com/apache/trafficcontrol/pull/7375
Adds a page/view/form for generating ISO system images to TPv2
<hr/>
## Which Traffic Control components are affected by this PR?
- Traffic Portal (experimental v2)
## What is the best way to verify this PR?
Run the provided tests. You can't actually generate an ISO in any demo/testing/development environment we provide, so manual testing would be a pain (also why there's no e2e tests).
## PR submission checklist
- [x] This PR has tests
- [x] This PR has documentation
- [x] This PR has a CHANGELOG.md entry
- [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on a diff in pull request #7375: Basic ISO generation page/form
Posted by "shamrickus (via GitHub)" <gi...@apache.org>.
shamrickus commented on code in PR #7375:
URL: https://github.com/apache/trafficcontrol/pull/7375#discussion_r1138767415
##########
experimental/traffic-portal/src/app/core/core.module.ts:
##########
@@ -74,6 +75,7 @@ export const ROUTES: Routes = [
{ component: TenantDetailsComponent, path: "tenants/:id"},
{ component: PhysLocDetailComponent, path: "phys-locs/:id" },
{ component: PhysLocTableComponent, path: "phys-locs" },
+ { component: ISOGenerationFormComponent, path: "gen-iso"}
Review Comment:
The sidebar has this as `iso-gen` meaning it can't be navigated to from there
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7375: Basic ISO generation page/form
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7375:
URL: https://github.com/apache/trafficcontrol/pull/7375#discussion_r1145272008
##########
experimental/traffic-portal/src/app/shared/file-utils.service.ts:
##########
@@ -0,0 +1,147 @@
+/**
+ * @license Apache-2.0
+ * 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.
+ */
+import { Inject, Injectable } from "@angular/core";
+
+import { isArrayBufferView } from "../utils";
+
+/**
+ * FileUtilsService provides utilities dealing with files, uploads, downloads,
+ * etc.
+ */
+@Injectable()
+export class FileUtilsService {
+
+ /** The default MIME-Type for string data downloads. */
+ public static readonly TEXT_CONTENT_TYPE = "text/plain;charset=UTF-8";
+ /** The default MIME-Type for raw binary data downloads. */
+ public static readonly BINARY_DATA_CONTENT_TYPE = "application/octet-stream";
+ /** The default MIME-Type for arbitrary object downloads. */
+ public static readonly JSON_DATA_CONTENT_TYPE = "application/json";
+
+ /**
+ * The file name that will be used for downloads, if one is not provided.
+ */
+ public static readonly DEFAULT_FILE_NAME = "download";
+
+ /** A pre-compiled expression that matches a '.' at the start of a string */
+ private static readonly EXT_PATTERN = /^\./;
+
+ constructor(@Inject("Window") private readonly window: Window) { }
Review Comment:
hm, I didn't even consider SSR limitations, since it's a lazy-loaded module. Regardless, fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7375: Basic ISO generation page/form
Posted by "ocket8888 (via GitHub)" <gi...@apache.org>.
ocket8888 commented on code in PR #7375:
URL: https://github.com/apache/trafficcontrol/pull/7375#discussion_r1139159602
##########
experimental/traffic-portal/src/app/core/core.module.ts:
##########
@@ -74,6 +75,7 @@ export const ROUTES: Routes = [
{ component: TenantDetailsComponent, path: "tenants/:id"},
{ component: PhysLocDetailComponent, path: "phys-locs/:id" },
{ component: PhysLocTableComponent, path: "phys-locs" },
+ { component: ISOGenerationFormComponent, path: "gen-iso"}
Review Comment:
:facepalm:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus merged pull request #7375: Basic ISO generation page/form
Posted by "shamrickus (via GitHub)" <gi...@apache.org>.
shamrickus merged PR #7375:
URL: https://github.com/apache/trafficcontrol/pull/7375
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficcontrol] shamrickus commented on a diff in pull request #7375: Basic ISO generation page/form
Posted by "shamrickus (via GitHub)" <gi...@apache.org>.
shamrickus commented on code in PR #7375:
URL: https://github.com/apache/trafficcontrol/pull/7375#discussion_r1144968673
##########
experimental/traffic-portal/src/app/shared/file-utils.service.ts:
##########
@@ -0,0 +1,147 @@
+/**
+ * @license Apache-2.0
+ * 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.
+ */
+import { Inject, Injectable } from "@angular/core";
+
+import { isArrayBufferView } from "../utils";
+
+/**
+ * FileUtilsService provides utilities dealing with files, uploads, downloads,
+ * etc.
+ */
+@Injectable()
+export class FileUtilsService {
+
+ /** The default MIME-Type for string data downloads. */
+ public static readonly TEXT_CONTENT_TYPE = "text/plain;charset=UTF-8";
+ /** The default MIME-Type for raw binary data downloads. */
+ public static readonly BINARY_DATA_CONTENT_TYPE = "application/octet-stream";
+ /** The default MIME-Type for arbitrary object downloads. */
+ public static readonly JSON_DATA_CONTENT_TYPE = "application/json";
+
+ /**
+ * The file name that will be used for downloads, if one is not provided.
+ */
+ public static readonly DEFAULT_FILE_NAME = "download";
+
+ /** A pre-compiled expression that matches a '.' at the start of a string */
+ private static readonly EXT_PATTERN = /^\./;
+
+ constructor(@Inject("Window") private readonly window: Window) { }
Review Comment:
E2E tests are failing cause the angular server is halting on `window is undefined`. I don't think you can actually inject the window like this and instead should do what is done in the `theme-manager.service.ts`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org