Skip to content
This repository has been archived by the owner on Apr 17, 2023. It is now read-only.

fix: add serve #186

Closed
wants to merge 1 commit into from
Closed

fix: add serve #186

wants to merge 1 commit into from

Conversation

wtrocki
Copy link
Contributor

@wtrocki wtrocki commented Jan 28, 2020

Add very basic cli that will help us to run server using model file

@wtrocki wtrocki requested review from b1zzu and darahayes January 28, 2020 12:18
import { TestxServer } from '../src';

async function startServer(model: string) {
const server = new TestxServer({
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to assign a port instead of having one randomly generated?

Copy link
Contributor Author

@wtrocki wtrocki Jan 28, 2020

Choose a reason for hiding this comment

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

+100.

Copy link
Contributor

Choose a reason for hiding this comment

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

The server start() method should be already abel to accpet a custom port, and if not is very easy to do it

Copy link
Contributor

Choose a reason for hiding this comment

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

@wtrocki if you can take do something like port = argv.port || '4001' and then call it a day

Copy link
Contributor

Choose a reason for hiding this comment

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

The TestxServer start is calling the GraphbackServer start function which accepts a fixed port, is just a matter of adding the argument to the TestxServer start and pass it through

await this.server.start();

public async start(port?: number): Promise<void> {

const modelLocation = argv.model as string;
if (existsSync(modelLocation)) {
const fileContent = readFileSync(modelLocation, { encoding: 'utf8' });
startServer(fileContent);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
startServer(fileContent);
startServer(fileContent).catch(e => throw e);

an alternative would be to log the error and stack and then exit with something different from 0, the only problem with the current way is that nodejs will give an error like not handled exception in promise which is not so nice

Copy link
Contributor

@b1zzu b1zzu Jan 28, 2020

Choose a reason for hiding this comment

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

the best approach will be to create a main function

async function main() {
  // everything here
}

main().catch(e => {throw e});

@b1zzu
Copy link
Contributor

b1zzu commented Jan 28, 2020

@wtrocki this is really great

@wtrocki
Copy link
Contributor Author

wtrocki commented Jan 28, 2020

This is just example what we can do. I do not intent to merge it here.
I was thinking about separate package. More info here:

aerogear/graphback#659

if (argv.model) {
const modelLocation = argv.model as string;
if (existsSync(modelLocation)) {
const fileContent = readFileSync(modelLocation, { encoding: 'utf8' });
Copy link
Contributor

Choose a reason for hiding this comment

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

What about moving the file read part inside Testx itself?
#97

@wtrocki wtrocki closed this Nov 8, 2022
@wtrocki wtrocki deleted the cli-for-runtime branch November 8, 2022 21:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants