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

Removing "awaiting response" label from adapters #35

Closed
dbeatty10 opened this issue Sep 23, 2022 · 6 comments · Fixed by #41 or dbt-labs/dbt-core#6120
Closed

Removing "awaiting response" label from adapters #35

dbeatty10 opened this issue Sep 23, 2022 · 6 comments · Fixed by #41 or dbt-labs/dbt-core#6120

Comments

@dbeatty10
Copy link
Contributor

this action needs to be copied over to the adapter plugin repos - it's just in dbt-core right now
or moved into the centralized actions repo, and then pulled into the others

Quickest solution

Copy this action over to each adapter plugin repos

Best solution

  1. Add this action to the centralized actions repo
  2. Pull it into dbt-core + each adapter
  3. Remove the original action from dbt-core
@emmyoop
Copy link
Member

emmyoop commented Sep 23, 2022

First, 100% agree this needs to be added to adapters. That's not the purpose of my comment. My comment is more generally process related. Tagging @leahwicz and @lostmygithubaccount to see if they have opinions here.

This is a good time to have the discussion on when to centralize actions. This action is just a single step job to a marketplace action

jobs:
  triage_label:
    if: contains(github.event.issue.labels.*.name, 'awaiting_response')
    runs-on: ubuntu-latest
    steps:
      - name: initial labeling
        uses: andymckay/labeler@master
        with:
          add-labels: "triage"
          remove-labels: "awaiting_response"

It may be worth centralizing so that if we change the labels for some reason we do it in only one place. Or if we update the version of the actions we want to use (we really shouldn't blindly point to master unless it's an action we control internally) we could do it in a single place.

But do we want to centralize workflows that are single step workflows like this? There are others that we could centralize that I have not because I was trying to avoid layering too deep but I'm not sure that's a great reason. When you make it a reusable workflow the workflow steps show in your local run so all the debug info is available, unlike when you're using a marketplace action and it's a bit of a blackbox.

After typing this up, I'm leaning towards centralizing anything we use across core and adapters if for no other reason than to control the versions we're using in a singular place.

@emmyoop
Copy link
Member

emmyoop commented Sep 23, 2022

Noting here so it's not lost: If we standardize on centralized actions, we should also pull the changie-bot action here and refer to it in all the adapters and core. There's also multiple issues open to update that action in all the adapters (minus spark which was already done).

@dbeatty10
Copy link
Contributor Author

Thanks for your analysis @emmyoop ! Definitely keen to support whatever your recommendation is.

@lostmygithubaccount
Copy link
Contributor

I'm not a huge fan of using random people's actions. all this is really doing is making a GitHub API request to change a label -- I think that can be done with the gh CLI pretty easily. should we consider forking/copying and maintaining actions like that ourselves?

I suspect that could help us more easily customize the behavior as we see fit too

even if we don't blindly point to master in an action we use and don't control, I don't think there's anything technically preventing a maintainer from making changes to a version we do pin to

@leahwicz
Copy link
Contributor

Couple points here:

  • I think the move to centralize all actions into a single repo when they are needed across core + adapters is the right move. No matter how big or small. It just give us a peace of mind that everything is the same. I wish there was a way to audit actions across repos but that isn't a thing yet
  • For using random people's actions- in most cases when we do this, we should evaluate the action before we use it. For example, who owns the action, how widely used is it, what does it but us, etc. In this case, I used andymckay's action b/c he works at GitHub and he worked on Action with me 😄.
  • We are looking to have a contractor come in and do some GitHub Actions work for us so centralizing these actions could be part of their responsibility. They also could come up with a framework on when to use a marketplace action or not so we have a standard to follow

@emmyoop
Copy link
Member

emmyoop commented Sep 23, 2022

I agree with @lostmygithubaccount - using the gh CLI is really easy and powerful, not to mention straight forward.

Some marketplace actions are super powerful though. Evaluating on a case by case basis is worthwhile. That should include actually looking at the code too. I have read that some places approach this by forking those actions and pointing to their own fork to fully control its version.

There is a security concern with pointing at master/main, but the more realistic/probable problem with blindly pointing to master is even if the action owner isn't being malicious, they may push broken code to master with no intention of releasing it and no intention of immediately fixing it since it's not an official release.

I suspect getting this specific action into the adapters might not wait until the contractor comes on since it effects monitoring triage. I definitely think it needs to be centralized here as part of the work to send it everywhere. It's not much more work than adding each one individually. Once it's centralized we can rework what it's doing specifically then if we want to wait for the contractor to help us make the best call there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants