Remove.wit.deployments.api.calls #3502
Conversation
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
@corinnekrych could you remove your package-lock files |
|
@christianvogt done. |
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
Ya we should add exclusion to the packages for now. When I install & bootstrap I do not get the package-lock generated in the sub dir. |
| @@ -72,13 +67,13 @@ export class AddAppOverlayComponent implements OnInit, OnDestroy { | |||
| switchMap((space) => { | |||
| if (space) { | |||
| this.currentSpace = space; | |||
| return this.deploymentApiService.getApplications(space.id); | |||
| return observableEmpty(); | |||
There was a problem hiding this comment.
This will now allow applications with the same name to be created. Is there any alternative way to do this?
| this.context.space ? this.context.space.id : '', | ||
| ); | ||
| getApplicationsInASpace(): Observable<any[]> { | ||
| return of([]) |
There was a problem hiding this comment.
@vikram-raj how is this used in launcher?
There was a problem hiding this comment.
@christianvogt It is use to validate the application name while creating the application. So, that user can not create multiple application with the same name.
Here it is used https://github.com/fabric8-launcher/ngx-launcher/blob/master/projects/ngx-launcher/src/lib/shared/project-name.validator.ts#L34
There was a problem hiding this comment.
@corinnekrych can we use the same workaround here to get the application names?
There was a problem hiding this comment.
@vikram-raj @christianvogt is that check really needed? I mean we already check that in add-app-overlay
There was a problem hiding this comment.
@corinnekrych getApplicationsInSpace is getting called in project-name.validator directive in ngx-launcher. https://github.com/fabric8-launcher/ngx-launcher/blob/master/projects/ngx-launcher/src/lib/shared/project-name.validator.ts#L34
There was a problem hiding this comment.
@edewit do you know why we used project-name.validator given that we've already checked the name is not already in used in
?There was a problem hiding this comment.
@corinnekrych Looks like the app launcher has two more areas after the initial screen where the user can change the application name:
…krych/fabric8-ui into remove.wit.deployments.api.calls
|
@christianvogt @vikram-raj using |
| @@ -216,15 +212,6 @@ describe('AddAppOverlayComponent', () => { | |||
| expect(component.applications).toEqual([]); | |||
| }); | |||
|
|
|||
| it('should retieve applications if the current space is defined', () => { | |||
There was a problem hiding this comment.
Can we keep these test cases now that you've found a work around to the list of applications.
| this.context.space ? this.context.space.id : '', | ||
| ); | ||
| getApplicationsInASpace(): Observable<any[]> { | ||
| return of([]) |
There was a problem hiding this comment.
@corinnekrych can we use the same workaround here to get the application names?
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
@corinnekrych , Using the |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
@christianvogt I put back the tests, unit tests are ok now but CI is failing with this: 🤔 #puzzled |
|
[test] |
|
Something is wrong now with local dev and login. I'm getting a I cannot login to the dev app. |
|
Looked into the login issue a bit further. This is caused by the @vikram-raj @rohitkrai03 could one of you help @corinnekrych in addressing this issue. Maybe somehow move the init code to only when the modal is actually needed rather than on app load. |
|
@christianvogt Will take a look at it in the morning. |
|
@christianvogt @rohitkrai03 indeed testing with a new incognito browser I can see the issue :( |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
@corinnekrych |
|
It's the initialization of the pipeline service through dependency injection that is the issue. Not the service call itself. |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
not to worry @edewit enjoy your ⛷ holidays, we got it sorted with, as Christian identified where it was used: @christianvogt ready for another review |
|
Sorry but I have no idea
…On Tue, 29 Jan 2019, 16:18 ckrych ***@***.*** wrote:
***@***.**** commented on this pull request.
------------------------------
In
packages/fabric8-ui/src/app/space/app-launcher/services/app-launcher-dependency-check.service.ts
<#3502 (comment)>
:
> @@ -39,9 +35,7 @@ export class AppLauncherDependencyCheckService implements DependencyCheckService
*
* @returns Observable
*/
- getApplicationsInASpace(): Observable<Application[]> {
- return this.deploymentApiService.getApplications(
- this.context.space ? this.context.space.id : '',
- );
+ getApplicationsInASpace(): Observable<any[]> {
+ return of([])
@edewit <https://github.com/edewit> do you know why we used
project-name.validator given that we've already checked the name is not
already in used in
https://github.com/fabric8-ui/fabric8-ui/blob/8ebda0a75136aaaa525d54da5c018e026ce01409/packages/fabric8-ui/src/app/space/add-app-overlay/add-app-overlay.component.ts#L165
?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3502 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AADHvf7dhdsM3UMvkr_Q4XgPZeWOZUjqks5vIGYvgaJpZM4aQdmn>
.
|
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
🎉 This PR is included in version 5.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
* clean(deployment): remove wit deployment api calls * fix(test): to match removal of api deployment * clean(build): remove package-lock files * refactor(deployment): use build config to get OS application name * refactor(add-app-overlay): add back unit test to check application name availability * fix(auth): delay injection of PipelinesService to fix auth loop * clean(app-launcher): put back name validation * clean(app-launcher): remove instance variable and send the observable direclty BREAKING CHANGE: all deployment code removed
Related to fixes https://openshift.io/openshiftio/Openshift_io/plan/detail/1315