Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NETOBSERV-316 Console plugin as standalone #163

Merged
merged 6 commits into from
Jun 7, 2022

Conversation

jpinsonneau
Copy link
Contributor

@jpinsonneau jpinsonneau commented May 23, 2022

This allow to run plugin as standalone. You can port-forward loki from anywhere or mock it.

Simply run make start-standalone to run backend + standalone frontend with live reloading
Or build-standalone then make serve.

Generated index.html + js files are hosted in backend at http://localhost:9001/

image
image

You can run cypress testing using this method. Simply update port number in consts.js

Will need #152 to support dark theme & implements ResourceLink icons. This can be added in another PR.

@openshift-ci
Copy link

openshift-ci bot commented May 23, 2022

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link
Collaborator

@OlivierCazade OlivierCazade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

I just had to run install-frontend before start-standalone to make it work.
Thanks!

Makefile Show resolved Hide resolved
@jpinsonneau
Copy link
Contributor Author

I just had to run install-frontend before start-standalone to make it work. Thanks!

Yes I can add this on both start / start-standalone 👍
Thanks for the feedback @OlivierCazade

@openshift-ci openshift-ci bot removed the lgtm label May 24, 2022
@OlivierCazade
Copy link
Collaborator

/lgtm

@openshift-ci openshift-ci bot added the lgtm label May 24, 2022
@eranra
Copy link
Contributor

eranra commented May 25, 2022

@jpinsonneau just ran that on my machine ... looks good to me. some remarks::

(1) the term install in the Makefile is a bit confusing ... it might be better to use something like deploy or/and build those terms are easier to understand and not ambiguous.

(2) For completeness, it might be nice to have some mock Loki (or simulation) of data so that when you run the plug-in without real data we will be able to show something.

(3) From looking on the screenshot - need to add the nice icons back :-)

@jpinsonneau
Copy link
Contributor Author

jpinsonneau commented May 25, 2022

(1) the term install in the Makefile is a bit confusing ... it might be better to use something like deploy or/and build those terms are easier to understand and not ambiguous.

This actually come from npm install. We can add dependencies keyword as same as in the echo command or maybe npm mention ?

.PHONY: install-frontend
install-frontend:
	@echo "### Installing frontend dependencies"
	cd web && npm ${NPM_INSTALL}

Or remove these and run npm install directly. What would you prefer ?

(2) For completeness, it might be nice to have some mock Loki (or simulation) of data so that when you run the plug-in without real data we will be able to show something.

I'm on it 👍 currently we have the json-server but the jsons are outdated. I'm also looking if we can have something better like reusing mocks from tests or restore a loki storage from previous saved data.

(3) From looking on the screenshot - need to add the nice icons back :-)

This will be done after #152 merge

Thanks for the feedback @eranra !

@openshift-ci
Copy link

openshift-ci bot commented Jun 1, 2022

New changes are detected. LGTM label has been removed.

@openshift-ci
Copy link

openshift-ci bot commented Jun 1, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from eranra after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jpinsonneau
Copy link
Contributor Author

jpinsonneau commented Jun 1, 2022

Rebased and added missing resource icons:

image

It seems there are issues with button styles in dark 🤔 The PF colors are not dynamically updating on theme change and I can't find a workaround for now.

@jpinsonneau
Copy link
Contributor Author

Rebased to test with cypress and fixed lasts styling issues
Also added start-standalone-mock to run standalone with mocked loki backend

image
image
image
image
image

@jpinsonneau jpinsonneau merged commit e179ea5 into netobserv:main Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants