Skip to content

Feature: Add set up development environments docs#22

Merged
syphax-bouazzouni merged 12 commits intomasterfrom
feature/add-development-environment-guides
Mar 9, 2023
Merged

Feature: Add set up development environments docs#22
syphax-bouazzouni merged 12 commits intomasterfrom
feature/add-development-environment-guides

Conversation

@syphax-bouazzouni
Copy link
Collaborator

@syphax-bouazzouni syphax-bouazzouni commented Feb 3, 2023

Context

When we have new developers in the Alliance that don't just want to run the appliance but also want to do some development and enhancement to their local installation.

The problem is that it's hard to start a local development environment and understand how our code works.

This PR tries to give a draft version of two documentation pages:

  1. Ontoportal Architecture: That describes our architecture, you can see a preview here
  2. Set up a development environment: That gives two guides on how to set up a development environment for the front and backend; you can see a preview here

Changes

  • Update getting started content(b556b4b)
  • Add backend environment documentation page (8e00038)
  • Add frontend environment documentation page(185b9bc)
  • Update and move the architecture page(a762b3b)
  • Rename the old getting_started to debugging_ontoportal (d738590)
  • Update the developer section title (a606ce1)

@syphax-bouazzouni syphax-bouazzouni self-assigned this Feb 3, 2023
@syphax-bouazzouni syphax-bouazzouni added the documentation Improvements or additions to documentation label Feb 3, 2023
@syphax-bouazzouni
Copy link
Collaborator Author

@RaimiInfai here are the docs that we spoke about. As you are the ones currently setting up a new portal (BioDivPortal), your feedback is welcomed.

Same for you @Bouchemel-Nasreddine; hope this will help you set up your dev environment for your internship in the Industry portal.

And for the ones that want to edit or add something to this PR, collaboration is open just add your commits in the branch feature/add-development-environment-guides

Copy link

@stdotjohn stdotjohn left a comment

Choose a reason for hiding this comment

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

I think that this pull request should be approved. In fact I think that much of the approval process could be simplified for this type of pull request, anyone who goes to the trouble of doing a writeup should be allowed to add it. I think that the use of VirtualBox
is very interesting. Qemu utilities, for example, should allow us to convert such a VM to other platforms such as KVM or VMWare. I am interested in Docker at the moment not because it can be lightweight (my containers probably are not) but because such VMs are defined by text files that define their history.

My major confusion about this document is that at tne end it dafines how to run the target executable not how to "run" it "in" a ruby IDE (with, for example, its nice graphical representation of breakpoints) such as RubyMine. In particular, debugging a container containing a ruby executable has challenges (remote debug and/or sharing a volume with the host) which I am investigating now.

@syphax-bouazzouni syphax-bouazzouni changed the title Feature: Add set up development environments Feature: Add set up development environments docs Feb 6, 2023
@syphax-bouazzouni
Copy link
Collaborator Author

I think that this pull request should be approved. In fact, I think that much of the approval process could be simplified for this type of pull request, anyone who goes to the trouble of doing a writeup should be allowed to add it.

The pull request approval process may slow down the contributions. But it's important to be transparent about our changes and let (all related) people know about it.

But yeah, maybe we can improve it, firstly to not wait for the approval of everyone, and just have a limited number of approval before merging (e.g 2 approval per PR). (we will see in the future, but for now, it doesn't bother me to wait)

My major confusion about this document is that at the end it defines how to run the target executable not how to "run" it "in" a ruby IDE (with, for example, its nice graphical representation of breakpoints) such as RubyMine. In particular, debugging a container containing a ruby executable has challenges (remote debugging and/or sharing a volume with the host) which I am investigating now.

The problem with IDEs like RubyMine is that it is not free and that everyone has their preferred IDE (most people use for example VS Code because it's free and lightweight).

So here I tried to make something, that will work everywhere for free.

@syphax-bouazzouni syphax-bouazzouni requested review from Bouchemel-Nasreddine and removed request for arsarkar February 6, 2023 16:07
Copy link
Collaborator

@graybeal graybeal left a comment

Choose a reason for hiding this comment

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

Does the existence of passwords (and perhaps other config details) in this documentation (in the config file) create a security issue that can be mitigated in any way?

@graybeal
Copy link
Collaborator

graybeal commented Feb 8, 2023

I agree with @stdotjohn in that pull requests for documentation can be assumed to be 'likely better than what came before', if we believe in best intentions of the submitter. For example I found 2 typos in this one but assessed that it was far better to install it as is, than to not get my approval or delay merging because I didn't have time to recommend those changes. And small issues are easy to fix later. So if the first reviewer is doing a reasonably thorough review, and approves, I think that is enough; the rest of the reviews are gravy.

But if you merge a pull request after m reviews, do the n-m reviewers who are left still have the option to review it? Ideally they would have the option and would exercise it, because yes the dev teams should have an idea what is changing 'under them'.

@stdotjohn
Copy link

Does the existence of passwords (and perhaps other config details) in this documentation (in the config file) create a security issue that can be mitigated in any way?

I don't really think that this poses a development challenge. First of all, for an attacker to get onto a development machine at all is a serious security breach. A typical. way that this could happen is when an attacker sends the victim an attachment that the victim clicks on. (There are many other ways.). A developer depends on NAT and firewalls and other such mechanisms to protect herself.

In deployment such issues are very serious however.


$BIOMIXER_URL = "http://#{$HOSTNAME}:8081/BioMixer"

$ANNOTATOR_URL = $PROXY_URL = "http://#{$HOSTNAME}:8081/annotator"
Copy link

Choose a reason for hiding this comment

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

To get this work with default settings I used #{REST_URL} instead of http://#{$HOSTNAME}:8081

Copy link

Choose a reason for hiding this comment

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

This applies also to the other lines involving http://#{$HOSTNAME}:8081

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Did you put your appliance IP in $HOSTAME instead of localhost

@alexskr
Copy link
Collaborator

alexskr commented Mar 2, 2023

relying on the ontoportal appliance VM for running API for development purposes can be cumbersome. So the question is should we make some effort to replace it with recently published docker-based API workflows or add that as a second option?

@syphax-bouazzouni
Copy link
Collaborator Author

Hi @alexskr.

Using the new docker-based environment would be, better.

I propose the following roadmap:

  1. First merge this first version (to not complexify this PR)
  2. Secondly you can create another PR, to add as a recommended option to use the docker-based option (as a transition for teams/people that are already using the appliance-based env)
  3. When we do the transition in our teams (and test it for our local code), I will do a last PR to remove the second option.

@syphax-bouazzouni syphax-bouazzouni merged commit d492d26 into master Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants