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

Issue#53 - Update Hydra's environment variables #54

Merged
merged 10 commits into from
Nov 26, 2019

Conversation

claudiosegala
Copy link
Contributor

@claudiosegala claudiosegala commented Nov 22, 2019

Fix #53, start #12 and some minor improvements.

@claudiosegala claudiosegala changed the title Issue#53 Issue#53 - Update Hydra's environment variables Nov 22, 2019
@claudiosegala
Copy link
Contributor Author

@abilioesteves do you know any way to stress test every variable?

@eabili0
Copy link
Contributor

eabili0 commented Nov 22, 2019

@claudiosegala, what do you mean?

@@ -87,13 +93,13 @@ From the project root folder, fire the following commands to execute this projec
--shutdown-time 10 \
```

4. Authorize application on hydra

5. Authorize application on hydra
Copy link
Contributor

Choose a reason for hiding this comment

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

@claudiosegala, now that we have an example web app, couldn’t we just remove sections 5 and 6 and just add the example app as dependency at the available docker-compose.yml file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@abilioesteves, isn't it better to just add the whisper-examples link, instead of replacing?

Copy link
Contributor

Choose a reason for hiding this comment

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

The example we created is much better than the default one from Hydra. We'll have a docker image available for the example, making it easy for future users to execute Whisper and get a taste for its functionalities. The less cognitive overhead we have on our documentation, the better users will understand how Whisper works. Could you please do it, @claudiosegala? I'll start a docker image generation pipeline for the Web app whisper example.

@claudiosegala
Copy link
Contributor Author

@claudiosegala, what do you mean?

I mean that I change the variables and from what I saw, it seemed to work. But I do not think that the tests I made covered the usages of every variable that I changed.

@eabili0
Copy link
Contributor

eabili0 commented Nov 25, 2019

@claudiosegala, I think we should open a new issue to discuss how to test all the important variables (so Whisper can work nicely with Hydra). It's a complicated subject, since we are talking about integration testing (not unit testing). We should then discuss how to proceed with the creation of an Integration Testing pipeline, and then stress-out these variables.

@claudiosegala
Copy link
Contributor Author

@abilioesteves, I agree with you. Should I open this issue?

@eabili0
Copy link
Contributor

eabili0 commented Nov 25, 2019

@claudiosegala, yes, please

@eabili0 eabili0 merged commit d18158e into labbsr0x:master Nov 26, 2019
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.

Update Hydra's environment variables
2 participants