search

Kitware / dive

Media annotation and analysis tools for web and desktop. Get started at https://viame.kitware.com
https://kitware.github.io/dive
Apache License 2.0
82 stars 21 forks source link

Set up Netlify deploy previews #274

Open jjnesbitt opened 4 years ago

jjnesbitt commented 4 years ago

Now that https://github.com/girder/girder/pull/3300 is merged, the capability exists to automatically build previews for the client through Netlify, so that we can see client changes without having to spin up locally. However, this won't cover the case of changed client functionality that depends on server changes, as it will be pointing to the deployed application.

Depends on #1116

subdavis commented 4 years ago

CORS isn't set up for this app: Girder expects requests to originate from viame.kitware.com and not netlify.

Also, deploying mismatched client/server could cause inconsistencies in our data model if the server isn't defensive enough about what values it accepts. Bryon and I just found a problem with this.

jjnesbitt commented 4 years ago

CORS isn't set up for this app: Girder expects requests to originate from viame.kitware.com and not netlify.

I think I know what you mean by this. However, I've done some local testing and I believe our client configuration can be changed to allow for this change. Perhaps we could discuss this offline.

Also, deploying mismatched client/server could cause inconsistencies in our data model if the server isn't defensive enough about what values it accepts. Bryon and I just found a problem with this.

I agree this is a valid concern. If we think building deploy previews is dangerous in the presence of any server changes, we could create a custom rule to ignore preview builds if the server folder contains changes.

subdavis commented 4 years ago

Oh yeah, we could configure cors, I just meant that we hadn't done that yet.

Once we fix the other bug, we might actually be OK, I just haven't looked yet

The rule thing is cool, I was not aware of that

subdavis commented 4 years ago

I've been thinking about this some more, Jake, and I think you're right that we need Netlify deploy previews, but I don't think that's the whole issue.

Feature velocity / acceptance criteria

If we had retrospectives, I'd bring this up then. But I've noticed that our workflow may be a little flawed around the done/archived step. Matt is serving as our product owner, so he alone should get to decide whether or not a ticket has been done properly. Our deploys lag master, so he isn't able to do this, which means he has been rubber-stamping stuff then coming back a week or two later with follow-on issues that really should have blocked the ticket from being closed.

In other words, I think our feature velocity may be a bit too high and acceptance criteria too lose at the moment, which is causing QA-type issues that are impacting production and customers.

Staging env

If we had a full staging environment with automated deployments, and netlify previews pointed at staging instead of prod, I think our workflow hiccups would be solved. Matt could immediately look at a feature and decide whether it passes or fails the acceptance criteria.

I don't think we need another server for this. We could run the staging environment alongside prod for now. I recognize that isn't a best practice, but I don't think we have enough user traffic to justify multiple servers. Even something dumb like putting staging on a different port would be fine with me.

Then, our netlify previews could be pointed at the staging instance and if we break stuff, it doesn't matter.

Thoughts?

jjnesbitt commented 4 years ago

I think having a staging instance is a good idea, and it would solve the issue you mentioned around mismatched data models, but it raises some questions.

For one, we'd be in the same situation that we're in now regarding deploy timing, as we don't want to have open ports to listen for webhooks, so we'd need to run the deploy on a timer (meaning changes to master wouldn't be instantly deployed). I guess we'd just run the job for staging more frequently, which might not be an issue, just inconvenient.

The other concern I have is resources on the server's GPUs. I wonder how much extra strain a staging instance would put on those GPUs, when really we expect them to both be allocated almost entirely to the main instance. Although the amount of people using staging would be drastically lower, running training/pipelines on staging would still use up significant GPU resources.

I do think having a staging instance is a good idea, I just want to make sure we've considered these things ahead of time.

waxlamp commented 2 years ago

@subdavis I think this is a great idea in general, though I'm seeking your thoughts on how much ROI we'd get from doing it

  1. right away (without being defensive on the server)
  2. after setting up a staging server (which eliminates the danger of corrupting data)

If neither of those options justifies the investment, we can close this issue.

subdavis commented 2 years ago

RE: Staging server on prod hardware: That's a dumb idea and August Brandon was wrong. RE: Out-of-sync client: I'm still concerned about this, but not that much. We have pydantic for most json validation. Having a netlify preview of the client pointed at prod seems reasonable and safe to me.

This is still a valid issue, but probably low priority.