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

declare image_tag argument in Dockerfile #537

Merged
merged 1 commit into from
Dec 1, 2021
Merged

declare image_tag argument in Dockerfile #537

merged 1 commit into from
Dec 1, 2021

Conversation

cjyetman
Copy link
Member

@cjyetman cjyetman commented Dec 1, 2021

Apparently you have to declare the build ARG for it to be picked up and available from the --build-arg image_tag="$tag" docker build option? Honestly I don't know much about this stuff, but this seems to be working as expected now. Easy way to test immediately after building an image is with docker inspect 2dii_pacta:[TAG_NUMBER] and check for something like this in the Config section

"Env": [
                "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
                "build_version=0.3.0"
            ]

declare image_tag argument in Dockerfile
80b8a9d 
Apparently you have to declare the build ARG for it to be picked up and available from the `--build-arg image_tag="$tag"` `docker build` option?
@cjyetman
Copy link
Member Author

cjyetman commented Dec 1, 2021 

cjyetman:pacta_web_template_v0.0.13 cj2dii$ ./run-like-constructiva-flags.sh -t latest -p Test_PA2021NO
Running Docker Container

-- web_tool_script_1.R ---------------------------------------------------------

-- Docker build v0.3.0 ---------------------------------------------------------

@cjyetman cjyetman merged commit 3f02896 into master Dec 1, 2021
@cjyetman cjyetman deleted the cjyetman-patch-1 branch December 1, 2021 22:40
cjyetman pushed a commit that referenced this pull request Dec 1, 2021
declare image_tag argument in Dockerfile
5eb5d9f 
as in #537
AlexAxthelm pushed a commit that referenced this pull request Dec 12, 2021
Squashed commit of the following:
46c60bc 
commit ad066dc
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Thu Dec 9 18:17:22 2021 +0100

    improve efficiency reducing and reordering layers (#546)

commit 6efa32b
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Tue Dec 7 22:28:57 2021 +0100

    revert coercion from #540 (#543)

    revert coercion from #540 because `NULL` is needed to fallback on the defaults and `as.character(NULL) == character(0)`

commit 3dd308e
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Tue Dec 7 14:39:21 2021 +0100

    begin very minimal auto-testing of 0_portfolio_input_check_functions.R (#514)

commit 92e3f67
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Tue Dec 7 11:55:59 2021 +0100

    suppress annoying `{dplyr}` message during tests (#542)

commit 7af7781
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Mon Dec 6 09:55:51 2021 +0100

    remove unnecessary packages (#541)

commit e78e6c1
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Sun Dec 5 12:28:16 2021 +0100

    use the portfolio ID not the name for output dir (#539)

commit 66bbcd9
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Sun Dec 5 12:20:55 2021 +0100

    read in config values as strings (#540)

commit 3f02896
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Wed Dec 1 23:40:17 2021 +0100

    declare image_tag argument in Dockerfile (#537)

    Apparently you have to declare the build ARG for it to be picked up and available from the `--build-arg image_tag="$tag"` `docker build` option?

commit af9d8b6
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Wed Dec 1 20:44:21 2021 +0100

    Colombia PA2021CO should use 2020Q4 data (#521)

    force merging because no admin response in 7 days

commit b0537d7
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Wed Dec 1 20:43:26 2021 +0100

    update shell script to check the current set of necessary repos (#524)

    force merging because there's been no response in 7 days

commit bbd33a8
Merge: 82d6596 38eee72
Author: Alex Axthelm <Alex@2degrees-investing.org>
Date:   Wed Dec 1 17:34:08 2021 +0100

    Merge pull request #536 from 2DegreesInvesting/write-tag-inside-docker-build

    add build version as ENV var inside image and display it when running

commit 38eee72
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Wed Dec 1 17:09:52 2021 +0100

    use `Sys.getenv()`

commit 82d6596
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Wed Dec 1 16:51:07 2021 +0100

    turn TDM back on for GENERAL users (#523)

commit eef1ef7
Merge: e0f53d2 0cc7e83
Author: Alex Axthelm <Alex@2degrees-investing.org>
Date:   Wed Dec 1 15:19:04 2021 +0100

    Merge pull request #535 from 2DegreesInvesting/feature/better-error-handling

    quit wiith 0 status code, rather than error

commit 0cc7e83
Author: Alex Axthelm <alex@asset-resolution.com>
Date:   Wed Dec 1 14:35:52 2021 +0100

    change save and shutdown behavior

commit a71d712
Author: CJ Yetman <cj2dii@cjyetman.local>
Date:   Wed Dec 1 05:16:49 2021 +0100

    use function to return build version if available

commit ee6b5ea
Author: CJ Yetman <cj2dii@cjyetman.local>
Date:   Wed Dec 1 04:10:18 2021 +0100

    add build version as ENV var inside image and display it when running

commit 635113a
Author: Alex Axthelm <alex@asset-resolution.com>
Date:   Tue Nov 30 19:21:02 2021 +0100

    quit wiith 0 status code, rather than error

    the Constructiva server fails quietly (infinite spinner) if the docker
    container errors, but it will show the index.html if it returns status
    code `0`. So for user-correctable errors, we'll do that, and give them
    an informative message.

commit e0f53d2
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Tue Nov 30 16:01:33 2021 +0100

    genuinely represent the format that tm server uses (#534)

commit 477da47
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Mon Nov 29 23:41:26 2021 +0100

    rename `portfolio_name_in` to `portfolio_name` everywhere (#530)

commit 21187ad
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Mon Nov 29 23:03:16 2021 +0100

    reimplement `path_dropbox_2dii()` so we don't depend on `{r2dii.utils}` (#532)

commit e46ce20
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Mon Nov 29 15:02:14 2021 +0100

    update exchange rate for PA2021NO (#529)

commit 5d1f1d3
Author: CJ Yetman <cj2dii@cjyetman.local>
Date:   Mon Nov 29 00:31:34 2021 +0100

    revert unintended merge to master

commit c3f6231
Author: CJ Yetman <cj2dii@cjyetman.local>
Date:   Mon Nov 29 00:29:29 2021 +0100

    switch to using rocker/verse as base for transitionmonitor Docker

commit 3efbf59
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Thu Nov 25 13:37:10 2021 +0100

    update README with current dependencies (#522)

commit f6118af
Author: Jackson Hoffart <jackson.hoffart@gmail.com>
Date:   Wed Nov 24 15:32:11 2021 +0100

    Remove global env variables from `tdm_utility_functions` (#519)

commit e8ebbab
Merge: e40ac19 b333c06
Author: Alex Axthelm <Alex@2degrees-investing.org>
Date:   Wed Nov 24 14:22:50 2021 +0100

    Merge pull request #520 from 2DegreesInvesting/feature/hide-TDM

    Always evaluate tdm conditions as unmet

commit b333c06
Author: Alex Axthelm <alex@asset-resolution.com>
Date:   Wed Nov 24 14:07:36 2021 +0100

    Always evaluate tdm conditions as unmet

    this is a temporary workaround to allow a new docker image to be built
    and deployed, without rolling out TDM to users prematurely.

commit e40ac19
Author: Jackson Hoffart <jackson.hoffart@gmail.com>
Date:   Tue Nov 23 18:43:59 2021 +0100

    Add scenario benchmark calculation to `calculate_tdm()` function (#518)

commit f1df933
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Tue Nov 23 14:25:10 2021 +0100

    remove all the confusing extra example files (#517)

    All the extra example files under working_dir/ add more confusion than any help. This removes all of them but the 2 files that are essential for the "default" example to work (based on TestPortfolio_Input being explicitly set as the default here and in the other web tool scripts). The existing .gitignore file already ignores any new files that might be added to those directories so they cannot be committed by accident. The directory structure underneath working_dir/ is required, so those sub-directories are kept/protected by containing a .gitkeep.txt file, like this one. The only two files left then under working_dir/ are those that are essential for PACTA to run:
    working_dir/10_Parameter_File/TestPortfolio_Input_PortfolioParameters.yml
    working_dir/20_Raw_Inputs/TestPortfolio_Input.csv

commit 0936c1d
Merge: c47a995 1131511
Author: Alex Axthelm <Alex@2degrees-investing.org>
Date:   Tue Nov 23 12:04:27 2021 +0100

    Merge pull request #516 from 2DegreesInvesting/feature/add-testing-script

    Add a script for testing, can use local repos

commit 1131511
Author: Alex Axthelm <alex@asset-resolution.com>
Date:   Mon Nov 22 22:38:27 2021 +0100

    workaround argument parsing in `docker run`

commit bac9211
Author: Alex Axthelm <alex@asset-resolution.com>
Date:   Mon Nov 22 22:03:34 2021 +0100

    Add a script for testing, can use local repos

    This script runs the container like Constructiva does, but allows for
    the uses of local repos (using flags to specify which repo is which).
    not particularly elegant, but I've found it usesful in my testing work
    so far.

commit c47a995
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Mon Nov 22 15:18:50 2021 +0100

    Pass config files (#513)

commit 57d2352
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Mon Nov 22 14:27:54 2021 +0100

    pass tdm data to create interactive report (#512)

commit cd34198
Author: Alex Axthelm <Alex@2degrees-investing.org>
Date:   Mon Nov 22 14:20:22 2021 +0100

    Add infrastructure for HTML user error reports (#509)

    This commit adds a lot to solve a relatively small problem, but opens up
    a better line of communication with users in the future.

    Generally, this adds log_user_errors() and prepare_error_report(),
    which allow us to identify user-level errors, which they can fix
    before running a new analysis.

    Specifically, this checks that the port_holdings_date in the
    portfolio parameters file is a scalar, not an array (which can happen if
    the user groups portfolios with different holding dates.)

commit 5e479be
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Mon Nov 22 13:49:06 2021 +0100

    integrate new TDM function into PACTA process (#511)

commit c1cdac0
Author: Jackson Hoffart <jackson.hoffart@gmail.com>
Date:   Fri Nov 19 22:01:41 2021 +0100

    `PACTA.analysis` gains `calculate_tdm()` (#506)

    Co-authored-by: CJ Yetman <cj2dii@cjyetman.local>

commit 048bf1e
Merge: 4725df0 b55edfd
Author: Alex Axthelm <Alex@2degrees-investing.org>
Date:   Tue Nov 16 16:37:17 2021 +0100

    Merge pull request #508 from 2DegreesInvesting/feature/start_date_from_holdings_date

    Add function to pull start date from holdings date

commit b55edfd
Author: Alex Axthelm <alex@asset-resolution.com>
Date:   Tue Nov 16 15:57:26 2021 +0100

    Add function to pull start date from holdings date

    if `start_date` not defined in project parameters, then use one from the
    `holdings_date` in portfolio parameters, with `2020L` fallback if neither is
    defined.

commit 4725df0
Merge: 704c7d7 0cbeaf9
Author: Alex Axthelm <Alex@2degrees-investing.org>
Date:   Tue Nov 16 09:55:08 2021 +0100

    Merge pull request #505 from 2DegreesInvesting/PA2021PE_2020Q4

    adapt the PA2021PE parameters to use the new 2020Q4 data

commit 704c7d7
Author: CJ Yetman <cj@2degrees-investing.org>
Date:   Mon Nov 15 12:48:05 2021 +0100

    enable reading port_holdings_date and setting data_location_ext with proper precedence (#507)

commit 0cbeaf9
Author: CJ Yetman <cj2dii@cjyetman.local>
Date:   Wed Nov 10 13:36:22 2021 +0100

    adapt the PA2021PE parameters to use the new 2020Q4 data
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@AlexAxthelm AlexAxthelm AlexAxthelm approved these changes

@maurolepore maurolepore Awaiting requested review from maurolepore

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cjyetman @AlexAxthelm