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

Use Conda environment file to manage frontend and backend dependencies #33

Merged
merged 48 commits into from
Aug 18, 2022

Conversation

lbianchi-lbl
Copy link
Contributor

Resolves: #13

TODO

  • Verify that the commands work on Windows

MichaelPesce and others added 27 commits July 13, 2022 17:52
Co-authored-by: Ludovico Bianchi <lbianchi@lbl.gov>
Co-authored-by: Ludovico Bianchi <lbianchi@lbl.gov>
Co-authored-by: Ludovico Bianchi <lbianchi@lbl.gov>
@lbianchi-lbl
Copy link
Contributor Author

lbianchi-lbl commented Aug 10, 2022

Note to self/for posterity: the Cypress error on Linux:

image

looks similar to this issue: cypress-io/cypress#21532

@lbianchi-lbl lbianchi-lbl marked this pull request as ready for review August 10, 2022 18:22
@lbianchi-lbl
Copy link
Contributor Author

@MichaelPesce While discussing this PR during the dev call I thought of a potential cause for the test failing on Windows, namely something connected with static assets (i.e. images, icons, standalone JS files, etc) not being found. This is just a guess at this point, but it could in principle be a cause according to the following criteria:

  • Why is it showing up only in this PR? This PR changes the way packages are installed (both Python and JS), which does affect where files are installed on the filesystem especially for the Python code
  • Why only on Windows? This is less clear, but it could be due to OS-specific differences such as path separators, symlinks, case-sensitiveness, etc

I'll look into this in more depth to try to address the failures.

@lbianchi-lbl
Copy link
Contributor Author

The changes that @MichaelPesce implemented for #35 seem to be working and the Windows checks are now passing! I'm switching this back to draft mode as I plan to add a few minor changes. We should plan on merging #35 first and then this PR.

@lbianchi-lbl lbianchi-lbl marked this pull request as draft August 11, 2022 22:09
Use Conda-compatible default shell


Remove redundant push job trigger
Use tight pinning for nodejs conda-forge package version
Use --prefix/--app-dir flags for startup of both backend and frontend


Revert usage of --prefix when invoking cypress
Conflicts:
	.github/workflows/main.yml
@lbianchi-lbl lbianchi-lbl marked this pull request as ready for review August 17, 2022 17:31
@lbianchi-lbl
Copy link
Contributor Author

@dangunter feel free to take a look (the most relevant changes are in README.md and the environment file I think) and let us know what you think. @MichaelPesce and I are ready to merge this whenever.

@ksbeattie ksbeattie merged commit 6b39116 into main Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:High High Priority Issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use a Conda environment.yml file to specify as many dependencies as possible
3 participants