Skip to content

Conversation

jturmelle
Copy link
Contributor

Modifications for more info on GitHub, updated conda instructions for shortfin/storm.

@aaron-kaplan aaron-kaplan self-requested a review January 14, 2025 16:13
Copy link
Collaborator

@aaron-kaplan aaron-kaplan left a comment

Choose a reason for hiding this comment

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

I suggest the following top-level structure:

  • Setting up a development environment on the server (skip the environment creation part).
  • Running the application in your development environment
  • Modifying the conda environment (only for DL team, including steps to update the shared conda environment)
  • Managing datasets
    • Adding new datasets (to be written)
    • Updating existing datasets
    • Adding foreign tables (to be merged into "adding new datasets" once the whole process of adding shape data is documented)

Perhaps we should split "Managing datasets" into a separate document?

### On the server, create an ssh key to use with GitHub
## GitHub Account

The repository is public, so unless you are making changes to the code, you don't need a github account.
Copy link
Collaborator

Choose a reason for hiding this comment

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

After reading further, I see that for users without a github account you give the instructions for cloning over https further down, but when I first read to here it wasn't clear that that would be coming. I still would have followed the instructions to create an ssh key, and then I would have gotten stuck at "Configure GitHub to use that ssh key."

I think most readers of this document will have a github account already. Have you encountered anyone who didn't? I'm hesitant to complicate things by introducing a variant workflow for users that may not even exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Setting up a GitHub account and key always seems to elicit problems. The number of times we've spent the first 45 minutes getting them set up with git repositories is not small. I would argue setting up git shouldn't even be in this document.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The reason they need to run a test server is so they can test configuration changes before commiting and pushing them. So even if they manage to get a test server running without a github account, they need a github account for the next step.

5. Grant select privileges to the local user if necessary

GRANT SELECT ON table_name TO dero;
1. Login in the pgdb12 server using psql. Make sure the user/role you use has the appropriate permissionspsql -h pgdb12.iri.columbia.edu -U fist -W DesignEngine
Copy link
Collaborator

Choose a reason for hiding this comment

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

You seem to have made some unintended changes in this section.

@@ -1,66 +1,118 @@
# FbF Maproom
# Setting up a development FBF Maproom
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this change is appropriate. This is the title for the whole document, which includes not only setting up a development environment, but also updating datasets, modifying the python environment, and adding a foreign table.


Note that the command is `conda create`, not `conda env create`. Both exist, and they're different :-(

### Create a local configuration file
Copy link
Collaborator

Choose a reason for hiding this comment

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

A pre-existing problem: this is part of the "Python miniconda environment" section, but has nothing to do with miniconda. Should be bumped out one level.

@jturmelle
Copy link
Contributor Author

Maybe we move all of this out to the Wiki?

@aaron-kaplan
Copy link
Collaborator

Maybe we move all of this out to the Wiki?

Sounds good.

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.

2 participants