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

Add PR deployment preview #2374

Open
3 tasks done
mtrezza opened this issue Jan 27, 2023 · 15 comments · Fixed by #2364 or #2377
Open
3 tasks done

Add PR deployment preview #2374

mtrezza opened this issue Jan 27, 2023 · 15 comments · Fixed by #2364 or #2377
Labels
type:ci CI related issue

Comments

@mtrezza
Copy link
Member

mtrezza commented Jan 27, 2023

New Feature / Enhancement Checklist

Current Limitation

Reviewing a dashboard PR requires more effort than PRs from other repos because

  • Parse Dashboard is a visual product with UI
  • We don't have UI testing as part of the CI

The dashboard PR has to be cloned locally in a development environment to see the effect of most PRs.

Feature / Enhancement Description

Add a preview deployment to easily review and preview PRs online without having to clone the PR locally.

See https://docs.uffizzi.com/references/uffizzi-ci/#2-install-authorize-the-uffizzi-github-app

Example Use Case

n/a

Alternatives / Workarounds

Clone locally.

3rd Party References

n/a

Uffizzi Feedback

@parse-github-assistant
Copy link

parse-github-assistant bot commented Jan 27, 2023

Thanks for opening this issue!

  • 🎉 We are excited about your ideas for improvement!

@mtrezza
Copy link
Member Author

mtrezza commented Jan 27, 2023

@gadkins, @ShrutiC-git

I've opened this issue to the get Uffizzi integration working. #2364 has been merged.

I've tried existing PR #2370, closed and re-opened it. Uffizzi shows a deployment for that PR, but it didn't post a comment in the PR thread with the deployment URL. Does that only work for new PRs? Uffizzi posted a comment with the URL, so it looks good.

However, after dashboard login, when clicking on the server app, the screen appears blank. Could be a dashboard issue, I'll investigate.

@ShrutiC-git
Copy link
Contributor

thanks for surfacing that, @mtrezza.

I'm sorry you're seeing that issue, indeed seems to be rather weird. So far I can tell, I'm not seeing any obvious issue with the preview environment itself. Do pass along your findings and if we might need to make any modifications, please let me know!

@mtrezza
Copy link
Member Author

mtrezza commented Jan 27, 2023

(Moved feedback section to top comment)

@mtrezza
Copy link
Member Author

mtrezza commented Jan 27, 2023

@dblythy If you have a minute, maybe you could take a quick look at this. When you open the preview URL in #2370 (comment) and log in to the dashboard, then a blank screen appears. Debugging the browser it seems to point to #2363. Maybe parsing that specific URL there fails because of the way it's composed.

@mtrezza
Copy link
Member Author

mtrezza commented Jan 28, 2023

@ShrutiC-git should Uffizzi update the deployment when a new commit happens on a PR? I took a look and there was no sign of re-deployment in the Uffizzi console after commits in #2375.

@ShrutiC-git
Copy link
Contributor

@mtrezza, commented on #2375

I think we all could benefit from getting on a call and going through the process once, and also taking up issues the community is seeing. What do you think?

@mtrezza
Copy link
Member Author

mtrezza commented Jan 28, 2023

@gadkins you wrote:

Let me clarify Uffizzi behavior:

  • You do not need to close and reopen PRs every time you push a new commit
  • Uffizzi is receiving webhooks on every commit, so it will rebuild your application each time.
  • Each time it rebuilds a new commit, it will preserve the state from the previous build if volumes are mounted in the compose. This tells Uffizzi to preserve this data as long as this PR is open.
  • If you close a PR, Uffizzi will destroy the preview and all its state. If you reopen the PR, you will get an fresh environment with empty volumes and a new comment.
  • On new commits, Uffizzi will update the existing comment on the PR. It will not post a new comment. The multiple comments you are seeing are caused by opening and closing the PR several times.
  • You can see which commit is deployed in the Uffizzi Dashboard, along with the timestamp:
Screen Shot 2023-01-27 at 11 35 52 PM

Thanks for clarifying this. I think what we would benefit from is an update of the PR comment by Uffizzi to see when the deployment has finished, without having to go to the Uffizzi dashboard. It seems unintuitive to me that when reopening a PR a new Uffizzi comment is posted, but when making a commit, no new comment is posted and the existing comment isn't updated in any way.

I think what would be more intuitive:

  • a) Post 1 permanent comment and update it to indicate which commit has finished deploying
  • b) Post 1 comment below the deployed commit and delete any previous Uffizzi comments, which may be more comfortable because one doesn't have to compare any hashes to find out which commit is deployed

I think we all could benefit from getting on a call

Sure, you can reach out to me on Slack and we can schedule a call

@ShrutiC-git
Copy link
Contributor

hey, @mtrezza,

Thanks for clarifying this. I think what we would benefit from is an update of the PR comment by Uffizzi to see when the deployment has finished, without having to go to the Uffizzi dashboard. It seems unintuitive to me that when reopening a PR a new Uffizzi comment is posted, but when making a commit, no new comment is posted and the existing comment isn't updated in any way.

Absolutely, that is the desired behavior. Something might be off if it's not behaving that way for you. I'll write to you on Slack to schedule a call - we'll fix this so it's efficient and intuitive for the community.

@mtrezza mtrezza reopened this Jan 28, 2023
@ShrutiC-git
Copy link
Contributor

@mtrezza here is our community Slack channel for Uffizzi users if you would like to keep in touch with updates and/or post questions/queries.

@mtrezza mtrezza pinned this issue Feb 11, 2023
@mtrezza
Copy link
Member Author

mtrezza commented Feb 11, 2023

I'd like to say that the newer, more compact comment looks much better. We can already see that Uffizzi is a significant time saver when reviewing Parse Dashboard PRs; thanks @gadkins, @ShrutiC-git for approaching us and implementing this.

@ShrutiC-git
Copy link
Contributor

thanks for the feedback, @mtrezza - that is great to hear!

@gadkins
Copy link

gadkins commented Feb 11, 2023

Glad we can help your project

@mtrezza
Copy link
Member Author

mtrezza commented Mar 11, 2023

Do you have a concept for how to use a temporary config for a preview deployment?

See #2404, in order to test the feature in Parse Dashboard, the static config file needs to be changed. This cannot be done from in the UI. The challenge is that if the config file is deployed with the PR, then we may forget to remove it and accidentally release it.

@mtrezza
Copy link
Member Author

mtrezza commented May 25, 2023

@gadkins @ShrutiC-git Here is a feature suggestion that would help us a lot.

Currently, the deployment is only based on the repo content. A PR however may require a slightly different configuration to actually show the feature in action. Temporarily changing the default config and committing it in the PR is risky, because if we forget to remove it, the product will get shipped with a different default config.

A way to solve this could be adding that configuration file to the repo in a hidden directory where the file will be replaced during the uffizzi deployment. For example a file in ./.uffizzi/replace/src/config.yml could instruct Uffizzi to replace the existing file ./src/config.yml. This way there is no risk that a demo config will be shipped in the actual product. We can easily strip the config file as part of our release process. But even if it's shipped, it won't interfere with the actual product files.

@mtrezza mtrezza unpinned this issue Jun 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment