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

feat(agent): Agent/$upgrade operation #4613

Merged
merged 70 commits into from
Jun 22, 2024
Merged

feat(agent): Agent/$upgrade operation #4613

merged 70 commits into from
Jun 22, 2024

Conversation

ThatOneBro
Copy link
Member

@ThatOneBro ThatOneBro commented May 31, 2024 •
edited
Loading

Closes #3900
Closes #4225

@vercel Vercel
Copy link

vercel bot commented May 31, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
medplum-provider ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 21, 2024 8:30pm
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
medplum-app ⬜️ Ignored (Inspect) Visit Preview Jun 21, 2024 8:30pm
medplum-storybook ⬜️ Ignored (Inspect) Visit Preview Jun 21, 2024 8:30pm
medplum-www ⬜️ Ignored (Inspect) Visit Preview Jun 21, 2024 8:30pm

@ThatOneBro ThatOneBro self-assigned this May 31, 2024
@ThatOneBro ThatOneBro added this to the May 31st, 2024 milestone May 31, 2024
@reshmakh reshmakh modified the milestones: May 31st, 2024, June 30, 2024 Jun 2, 2024
@ThatOneBro
cleanup(agent): remove unused node-windows pkg
3e00acf
@ThatOneBro
wip(agent-upgrader): begining of upgrader
03dda11
@ThatOneBro
fix(agent-upgrader): fix how manifests are retrieved
162f18a
@ThatOneBro
test(agent-upgrader): test
ad67bb1
@ThatOneBro
wip(agent): $upgrade operation
f4014b7
@ThatOneBro
wip(agent): fix build errors
2ffff73
@ThatOneBro
wip(agent): add upgrader to installer
eee9a01
@ThatOneBro
wip(agent): fix reference to upgrader
3267532
@ThatOneBro
wip(agent): add `UPGRADER_FILE_NAME
bb06d60
@ThatOneBro
fix(agent-upgrader): also add upgrader when upgrading in installer
69924f2
@ThatOneBro
fix(logger): bind Logger methods to Logger context
8a472ab
@ThatOneBro
wip(agent): merge upgrader into agent
ef9d7c8
@ThatOneBro
wip(agent): fix build installer script
ea1491d
@ThatOneBro
wip(agent): fix IPC
722a299
@ThatOneBro
wip(agent): comment out version check
a620a0e
@ThatOneBro
wip(agent): change from \ to /
1e5f61e
@ThatOneBro
wip(agent): remove iconv-lite from external in agent
17eaf14
@ThatOneBro
wip(agent): add postject to devDeps
40bb6e1
@ThatOneBro
wip(agent): workaround isSea not working
d057935
@ThatOneBro
fix(package-lock): update
10cfe4d
@ThatOneBro
feat(app): add upgrade to ToolsPage.tsx
eff5cae
@ThatOneBro
fix(server/agent): fix \$upgrade route
640cab9
@ThatOneBro
fix(agent): actually call with --upgrade
16ca313
codyebberson
codyebberson reviewed Jun 18, 2024
View reviewed changes
Copy link
Member

@codyebberson codyebberson left a comment

Choose a reason for hiding this comment

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

Big picture looks good. See comments and questions. I'm going to test and run it on some windows machines before approval, but nothing else blocking.

Thanks for hard work on this, I imagine it was a slog 🙏

packages/fhir-router/src/fhirrouter.ts Outdated Show resolved Hide resolved
packages/agent/src/agent-main.ts Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason this has to be a separately file from main.ts? Kinda confusing to have main.ts and agent-main.ts.

Copy link
Member Author

@ThatOneBro ThatOneBro Jun 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

main.ts now is the entry point for two different "binaries"... I think it makes sense to split the main entry point from the entry point of each of the two paths...

Also when testing the main, I need to mock agentMain in order to test that it actually calls the right main between agentMain and upgraderMain, which is not possible if agentMain is in the same module as main itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

As a suggestion I'm not completely sold on but might bring more uniformity, should we rename agent-main.ts to agent.ts? So it's upgrader.ts and agent.ts ?

packages/agent/src/upgrader-utils.ts Outdated Show resolved Hide resolved
packages/agent/src/upgrader-utils.ts Outdated Show resolved Hide resolved
packages/app/src/resource/ToolsPage.test.tsx Show resolved Hide resolved
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Jun 21, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
93.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

@ThatOneBro ThatOneBro added this pull request to the merge queue Jun 22, 2024
Merged via the queue into main with commit 7fcfc60 Jun 22, 2024
32 checks passed
@ThatOneBro ThatOneBro deleted the derrick-agent-upgrade branch June 22, 2024 01:33
This was referenced Jun 22, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codyebberson codyebberson codyebberson approved these changes

Assignees

@ThatOneBro ThatOneBro

Labels
integration
Projects
Medplum Product Roadmap
Status: ✅ Done
Milestone
June 30, 2024
Development

Successfully merging this pull request may close these issues.

Find alternative to pkg for agent Medplum Agent Command: Force Upgrade
3 participants
@ThatOneBro @codyebberson @reshmakh