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.

Sign up for GitHub

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

Jump to bottom

Update dependencies to include real estate #361

Merged
merged 12 commits into from
Oct 22, 2024
Merged

Update dependencies to include real estate #361

merged 12 commits into from
Oct 22, 2024

Conversation

AlexAxthelm
Copy link
Collaborator

@AlexAxthelm AlexAxthelm commented Sep 26, 2024 •
edited
Loading

Not to be merged until dependencies are resolved.

Updates build configuration to use latest version of testing RE files.

Depends On:

@jdhoffa
Copy link
Member

jdhoffa commented Sep 26, 2024

@AlexAxthelm just flagging that I don't expect this to work, since the user_results directory will need to include the new real_estate directory? Unless you already added those on Azure.

See https://github.com/RMI-PACTA/user_results/pull/24/commits/1cebba7eb90f6d00e38c8d3a46e5e7da6c611a32

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 30, 2024 •
edited
Loading

Docker build status

commit_time git_sha project_code holdings_date language peer_group report summary image
2024-10-22T10:42:26Z f683c00, GENERAL 2022Q4 EN other Report transitionmonitordockerregistry.azurecr.io/rmi_pacta_2022q4_general:20241022T104545Z
2024-10-22T10:42:26Z f683c00, GENERAL 2023Q4 EN other Report transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_general:20241022T104544Z
2024-10-22T10:42:26Z f683c00, PA2024CH 2023Q4 DE bank Report Summary transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20241022T104544Z
2024-10-22T10:42:26Z f683c00, PA2024CH 2023Q4 DE other Report transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20241022T104544Z
2024-10-22T10:42:26Z f683c00, PA2024CH 2023Q4 EN assetmanager Report Summary transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20241022T104544Z
2024-10-22T10:42:26Z f683c00, PA2024CH 2023Q4 EN bank Report Summary transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20241022T104544Z
2024-10-22T10:42:26Z f683c00, PA2024CH 2023Q4 EN insurance Report Summary transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20241022T104544Z
2024-10-22T10:42:26Z f683c00, PA2024CH 2023Q4 EN other Report transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20241022T104544Z
2024-10-22T10:42:26Z f683c00, PA2024CH 2023Q4 EN pensionfund Report Summary transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20241022T104544Z
2024-10-22T10:42:26Z f683c00, PA2024CH 2023Q4 FR bank Report Summary transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20241022T104544Z
2024-10-22T10:42:26Z f683c00, PA2024CH 2023Q4 FR other Report transitionmonitordockerregistry.azurecr.io/rmi_pacta_2023q4_pa2024ch:20241022T104544Z
2024-10-22T10:42:26Z, f683c00, "ghcr.io/rmi-pacta/workflow.transition.monitor:pr-361"

@AlexAxthelm AlexAxthelm mentioned this pull request Oct 9, 2024
Merged
1 task
AlexAxthelm added a commit that referenced this pull request Oct 9, 2024
@AlexAxthelm
Restore Report Links (#364)
e47e031 
Updates test workflow to use new action to transfer files to/from Azure
Blob storage for persistence.

Depends On:
- [ ] RMI-PACTA/actions#109

Closes #363 

Note for reviewers: PA2024CH reports are being sent to a private blob
store as of this PR (changed in #361) so only the GENERAL links work
here.
@AlexAxthelm AlexAxthelm marked this pull request as ready for review October 11, 2024 13:29
@AlexAxthelm AlexAxthelm requested review from jdhoffa, MonikaFu and cjyetman and removed request for jdhoffa and MonikaFu October 11, 2024 13:29
@AlexAxthelm AlexAxthelm mentioned this pull request Oct 11, 2024
Merged
@jdhoffa
Copy link
Member

jdhoffa commented Oct 11, 2024

Again, I can't actually approve this, but looks good!

@MonikaFu
Copy link
Contributor

Are we using correct templates for German COP reports? I see a lot of 'CHAPTER TO BE REVIEWED FOR COP 2024' lines in there. Or do we still need a review of the German text? @jdhoffa maybe you are aware?

@MonikaFu
Copy link
Contributor

The table with links to real estate looks good to me. The only question I have is why some lines have a portfolio ID and some don't?

@AlexAxthelm
Copy link
Collaborator Author

LInes with a portfolio ID are for individual portfolios. Those without are for the participant-level (combined) reports.

@MonikaFu
Copy link
Contributor

thanks @AlexAxthelm . So apart from the templates question in German it looks good to me.

@jdhoffa
Copy link
Member

jdhoffa commented Oct 22, 2024

@MonikaFu can you please actually approve the PR :-)

@jdhoffa
Copy link
Member

jdhoffa commented Oct 22, 2024

I have followed up with Matthias regarding the DE content.

Regarding the RE content itself, FPRE will write that and circulate it before the end of the week.

MonikaFu
MonikaFu previously approved these changes Oct 22, 2024
View reviewed changes
@AlexAxthelm
Update references to main
f683c00 
following merge of related PRs
MonikaFu
MonikaFu approved these changes Oct 22, 2024
View reviewed changes
Copy link
Contributor

@MonikaFu MonikaFu left a comment

Choose a reason for hiding this comment

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

LGTM

@AlexAxthelm AlexAxthelm merged commit 551f797 into main Oct 22, 2024
22 checks passed
@AlexAxthelm AlexAxthelm deleted the real-estate branch October 22, 2024 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MonikaFu MonikaFu MonikaFu approved these changes

@cjyetman cjyetman Awaiting requested review from cjyetman

@jdhoffa jdhoffa Awaiting requested review from jdhoffa

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

Successfully merging this pull request may close these issues.
3 participants
@AlexAxthelm @jdhoffa @MonikaFu