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(deploy): pass jsonSchema #2406

Merged
merged 10 commits into from
Jun 27, 2024
Merged

fix(deploy): pass jsonSchema #2406

merged 10 commits into from
Jun 27, 2024

Conversation

bodinsamuel
Copy link
Contributor

@bodinsamuel bodinsamuel commented Jun 25, 2024

Describe your changes

Contributes to https://linear.app/nango/issue/NAN-1261/pass-json-schema-to-runner

  • Pass new property JsonSchema when deploying
    It's not used or stored yet.
    Added one test that assert the package() is correct and one integration test that checks deploy works.

  • Small rework of deploy function to remove params passed as reference

  • Revert the storage of raw undefined in model schema
    It's not compatible with JSON so it's now stored as a string instead (but still displayed correctly)

Tests

  • Try the UI and sync without deploying anything else
  • Try uploading with previous version nango deploy dev it shouldn't change anything
  • Try uploading with new version node packages/cli/dist/index.js deploy dev it shouldn't break anything :D

@bodinsamuel bodinsamuel self-assigned this Jun 25, 2024
Copy link

linear bot commented Jun 25, 2024

// sync_type: sync.sync_type as SyncType,
type: sync.type as any,
sync_type: sync.sync_type,
type: sync.type,
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the difference between sync_type and type and why it is uncommented here and not below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • type is action or sync
  • sync_type is full or incremental

it was commented by mistake, but it's not really used in the backend. Those two names are confusing but yeah legacy stuff

@@ -365,7 +358,7 @@ class DeployService {
return null;
}

return { flowConfigs: postData, postConnectionScriptsByProvider, jsonSchema: jsonSchema };
return { flowConfigs: postData, postConnectionScriptsByProvider, jsonSchema: JSON.parse(jsonSchema) as JSONSchema7 };
Copy link
Collaborator

Choose a reason for hiding this comment

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

parsing can fail. are we ok throwing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point

@bodinsamuel bodinsamuel merged commit e01e71e into master Jun 27, 2024
20 checks passed
@bodinsamuel bodinsamuel deleted the fix/deploy-send-jsonSchema branch June 27, 2024 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants