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

Finalize GitHub Sync #2352

Open
8 of 17 tasks
christad92 opened this issue Aug 4, 2024 · 4 comments
Open
8 of 17 tasks

Finalize GitHub Sync #2352

christad92 opened this issue Aug 4, 2024 · 4 comments
Assignees
Labels

Comments

@christad92
Copy link

christad92 commented Aug 4, 2024

Appetite: 5 days

  • Make "Initiate Github Sync" take the latest snapshots at the time of initiation

  • Support paths to job files in project.yaml files
    For users who work locally or for certain projects in their GitHub repos, users want to keep their jobs in different .js files and then have the files referenced in their project.yaml files. When OpenFn deploy is called, we need to make sure we get the content of the files and add them to the right section on the project.yaml.
    We should advise users to create a job.js file for every job such that for a workflow with 5 steps, we will need 5 jobs.js files. that are referenced in the .yaml file.

    • when using cli deploy and detecting a file path in the yaml, you merge in the real content before sending it to the lightning server
    • when using cli pull you get a new copy of the project.yaml and the projectState.json and compare it to the local copy of the project.yaml... you notice that the local project.yaml has file paths, and instead of committing the entirely new project.yaml to GitHub, you interpret the paths and write to those files.
    • if someone adds a new workflow or new steps to lightning directly, then syncs to GitHub for a project that already has relative paths for other workflows or jobs, they will retain the relative paths for things that are already set up locally, but the new workflow or job will be returned inline. (they can move it out later by hand if they want.)
    • on clicking export, keep it as a monolith for now. (out of scope to make export use either relative paths or monolith.)

See this example below, currently a job looks like this:

 Notify-CHW-upload-successful:
       name: Notify-CHW-upload-successful
       adaptor: @openfn/language-http@latest
       enabled: true
       credential:
       globals:
       body: |
         fn(state => state);
We'd expect users to set the body to the following, and for the deploy to still work. 

 Notify-CHW-upload-successful:
       name: Notify-CHW-upload-successful
       adaptor: @openfn/language-http@latest
       enabled: true
       credential:
       globals:
       body: Notify-CHW-upload-successful.js;

Notify-CHW-upload-successful.js should contain `fn(state => state)`;
  • Improve error handling and messaging for GitHub Integrations
    Since we are wrapping around GitHub APIs, we should be returning error messages that help users know how to diagnose and find solutions rather than returning generalized error messages like "Oops!...". The recommendation here will be that we don't process the error messages in any special manner but return it as GitHub Error: {message}. This way the user can try to search for solutions on their own or know what to communicate via the community.

Bugs to be fixed:

Nice to haves:
The following issues are nice to have and should not be considered a priority for this building cycle

Other consideration based on thunderbolt and plan limits.

@christad92 christad92 added this to v2 Aug 4, 2024
@github-project-automation github-project-automation bot moved this to New Issues in v2 Aug 4, 2024
@christad92 christad92 moved this from New Issues to Backlog in v2 Aug 4, 2024
@christad92 christad92 moved this from Backlog to Ready in v2 Aug 4, 2024
@christad92 christad92 added the epic label Aug 5, 2024
@stuartc
Copy link
Member

stuartc commented Aug 5, 2024

Instead of putting a path directly in the body key (which would also be a string value), we use this pattern instead:

  job-xyz:
    ...
    body:
      path: ../my-job.js

That way we can tell that the body is not available directly under the key much more easily, and leaves room for other features.

If the body is an object, and doesn't have a path key then we throw an error.

Note that this path is relative to the project.yaml, and not the current working directory.

@DrCord
Copy link

DrCord commented Aug 27, 2024

The link to the issue GitHub sync button available, but subscription doesn't allow github sync issue above is broken. I am interested in the issue, but cannot find it when searching.

I've found a related bug:
When the GitHub sync button is available without a subscription, while it does have an icon on hover that tells the user it they should not click it (circle with slash), it is still clickable and allows the user to go to GitHub to verify OAuth, which links back to the OpenFn instance on completion and then loads forever... However, if you refresh the page at that point, you get a page where GitHub sync says it's not enabled in the top banner, but you've got the options to select a GitHub installation - fairly confusing state too get a user into.
image

@christad92
Copy link
Author

#2462

@christad92 christad92 moved this from In progress to Backlog in v2 Sep 23, 2024
@taylordowns2000
Copy link
Member

Noting that after #2707 and #2462 @midigofrank and @stuartc will reasses this epic to determine if/what still needs to be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Backlog
Development

No branches or pull requests

5 participants