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

Document workflows #408

Closed
wants to merge 33 commits into from
Closed

Document workflows #408

wants to merge 33 commits into from

Conversation

maurolepore
Copy link
Contributor

@maurolepore maurolepore commented Feb 24, 2021

Building on work by Clare, this PR aims to document in README the two main workflows that the PACTA_analysis repo provides.

@maurolepore maurolepore changed the title Draft intro Document workflows Feb 25, 2021
@maurolepore maurolepore marked this pull request as ready for review March 3, 2021 04:17
@maurolepore maurolepore requested a review from cjyetman March 3, 2021 04:17
@maurolepore
Copy link
Contributor Author

maurolepore commented Mar 3, 2021

@cjyetman, if you have some time it would be great to have your opinion on this, my first try at documenting PACTA workflows in README. So far I only documented the so called "online" workflow; next I'll work on the "offline" workflow. (I propose to abandon the terms "online/offline workflow" in favor of "single-inputs/multiple-inputs workflow" or something else -- I explain why in README).

I recognize the most important workflow I have not documented yet. But I preferred to start with something more familiar to gain momentum.

https://github.com/2DegreesInvesting/PACTA_analysis/blob/generalize-workflow/README.md

@cjyetman
Copy link
Member

cjyetman commented Mar 3, 2021

I'm in favor of abandoning the "online"/"offline" terms, but I'm not in favor of abandoning having a term for the web tool scripts that clearly, unambiguously suggests that they are intended to work on the web server and must serve that purpose above all else. So I'm not sure "single-inputs" or "multiple-inputs" is adequate... in fact, it's not even clear to me which of those you would use to refer to the web tool scripts. My guess is "single-inputs" because it generally runs one portfolio at a time? But I would hope that the "offline" version would be capable of running a single portfolio, and I would prefer that if an analyst comes to this repo and says to themselves "I need to run PACTA on this single portfolio" that they're not then guided to use the web tools.

README.Rmd Outdated Show resolved Hide resolved
README.Rmd Show resolved Hide resolved
README.Rmd Show resolved Hide resolved
README.Rmd Outdated Show resolved Hide resolved
README.Rmd Outdated Show resolved Hide resolved
README.Rmd Outdated Show resolved Hide resolved
README.Rmd Outdated Show resolved Hide resolved
README.Rmd Outdated Show resolved Hide resolved
README.Rmd Outdated Show resolved Hide resolved
environment:
- PASSWORD=123
Copy link
Contributor

Choose a reason for hiding this comment

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

Blocking: These lines seem better suited to go into a docker-compose.override.yml, rather than the core docker-compose file.

docker-compose.yml Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docker/install.R Outdated

# Build an image with R packages from a specific date `snapshot`:
# docker build \
# --build-arg BUILD_DATE=yyyy-mm-dd \
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking: Do we know what CRAN's retention policy for old package binaries is?

README.Rmd Outdated Show resolved Hide resolved
@maurolepore
Copy link
Contributor Author

Blocking: These lines seem better suited to go into a docker-compose.override.yml, rather than the core docker-compose file.

Thanks! I'm new to compose and first time I learn about *overide.yml. Is this suggestion to (1) protect the PASSWORD (and we would exclude *overide.yml from the repo), or (2) to make it easier to reuse the app for other purposes?

If (1) I understand we could then do:

docker-compose -f docker-compose.yml -f /path/to/secrets/outside/repo/docker-compose.overide.yaml up

This is pretty cool but makes the call to up cumbersome. Is docker secret is a better approach (maybe harder to configure but easier start the container with docker-compose up)?

@maurolepore
Copy link
Contributor Author

BTW. I didn't anticipate that this PR would be about docker so I'll move the docker bits to a focused PR.

@AlexAxthelm
Copy link
Contributor

Is docker secret is a better approach

In my experience, unless there's a real secret, I find it's easiest to just put configuration stuff in a .env file, and keep that out of the repo (via .gitignore)

@maurolepore maurolepore marked this pull request as draft March 8, 2021 17:34
This was referenced Mar 8, 2021
@maurolepore
Copy link
Contributor Author

I'm closing this PR because it's too big and unfocused. I'll follow up with smaller more focused ones.

@maurolepore maurolepore closed this Mar 8, 2021
@cjyetman cjyetman deleted the generalize-workflow branch December 27, 2021 14:14
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