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

[FIX] Load scheduling DSL TypeScript libraries in scheduler-server #424

Merged
merged 3 commits into from
Nov 8, 2022

Conversation

pcrosemurgy
Copy link
Contributor

@pcrosemurgy pcrosemurgy commented Nov 7, 2022

  • Tickets addressed: None
  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

Since splitting scheduler-server into scheduler-worker – and thereby relocating the scheduling DSL Node project – the scheduler server has not had access to static DSL libraries. This PR loads scheduling DSL TypeScript libraries from the scheduler server JAR instead of requiring Docker to be configured to copy in the directory and set an environment variable.

Verification

  • Unit test added to test loading .ts files from the scheduler-server JAR.
  • Manually tested end-to-end.
  • Automated end-to-end tests.

Documentation

None.

Future work

None.

@pcrosemurgy pcrosemurgy temporarily deployed to e2e-test November 7, 2022 19:49 Inactive
@camargo camargo added the fix A bug fix label Nov 7, 2022
@pcrosemurgy pcrosemurgy marked this pull request as ready for review November 7, 2022 20:35
@pcrosemurgy pcrosemurgy requested a review from a team as a code owner November 7, 2022 20:35
@JoelCourtney
Copy link
Contributor

Manually tested end-to-end.

Can you automate it instead just for good measure? We already have a set of scheduler e2e tests that should do the busy work of uploading a model and creating goals.

@pcrosemurgy
Copy link
Contributor Author

@JoelCourtney Sure! I'll give it a shot.

Copy link
Member

@camargo camargo left a comment

Choose a reason for hiding this comment

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

Tested this branch against the UI e2e tests as well and all pass!

@pcrosemurgy
Copy link
Contributor Author

pcrosemurgy commented Nov 7, 2022

Not sure why this is failing:

1) src/tests/scheduler.test.ts:35:3 › Scheduling › Get scheduling DSL TypeScript =================

    ReferenceError: mission_model_id is not defined

      34 |
      35 |   test('Get scheduling DSL TypeScript', async ({ request }) => {
    > 36 |     typescriptFiles = await req.getSchedulingDslTypeScript(request, mission_model_id);
         |                                 ^
      37 |     expect(typescriptFiles).toBeDefined();
      38 |   });
      39 |

        at Object.getSchedulingDslTypeScript (file:///home/runner/work/***/***/e2e-tests/src/utilities/requests.ts:[176](https://github.com/NASA-AMMOS/aerie/actions/runs/3414650049/jobs/5682881013#step:13:177):97)
        at file:///home/runner/work/***/***/e2e-tests/src/tests/scheduler.test.ts:36:33

The test('Get scheduling DSL TypeScript', ...) comes right after test('Upload jar and create mission model', ...) which sets mission_model_id.

Edit: I think I've fixed this. The above error is strange as it makes it look like mission_model_id within the context of await req.getSchedulingDslTypeScript(request, mission_model_id); is undefined. However, it was actually undefined within utilities/requests.ts. The trace preview here is misleading IMO.

@pcrosemurgy pcrosemurgy force-pushed the fix/scheduler-server-dsl-libs branch from c58dc24 to acbe765 Compare November 7, 2022 22:52
@pcrosemurgy pcrosemurgy force-pushed the fix/scheduler-server-dsl-libs branch from acbe765 to 41ac03b Compare November 7, 2022 23:10
@pcrosemurgy pcrosemurgy force-pushed the fix/scheduler-server-dsl-libs branch from 41ac03b to 1c272a0 Compare November 7, 2022 23:23
@pcrosemurgy pcrosemurgy force-pushed the fix/scheduler-server-dsl-libs branch from 1c272a0 to f29b554 Compare November 7, 2022 23:43
@pcrosemurgy pcrosemurgy temporarily deployed to e2e-test November 7, 2022 23:43 Inactive
@pcrosemurgy pcrosemurgy force-pushed the fix/scheduler-server-dsl-libs branch from f29b554 to 7b85970 Compare November 7, 2022 23:48
@pcrosemurgy pcrosemurgy force-pushed the fix/scheduler-server-dsl-libs branch from 7b85970 to f9d5a87 Compare November 8, 2022 00:00
@pcrosemurgy pcrosemurgy temporarily deployed to e2e-test November 8, 2022 00:00 Inactive
@pcrosemurgy
Copy link
Contributor Author

Manually tested end-to-end.

Can you automate it instead just for good measure? We already have a set of scheduler e2e tests that should do the busy work of uploading a model and creating goals.

@JoelCourtney Done :)

Copy link
Collaborator

@mattdailis mattdailis left a comment

Choose a reason for hiding this comment

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

Did a quick manual check locally - intellisense is working in the scheduling goal editor again 🙏 Thanks for the quick turnaround, Paul 🎉

@camargo camargo merged commit 01cdc1a into develop Nov 8, 2022
@camargo camargo deleted the fix/scheduler-server-dsl-libs branch November 8, 2022 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix A bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants