packit / packit-service

Packit provided as a service
https://packit.dev
MIT License
34 stars 47 forks source link

Propose update event wrongly parsed for pre-releases #492

Closed puiterwijk closed 3 years ago

puiterwijk commented 4 years ago

I have made a new release v0.3 for Zezere (https://github.com/fedora-iot/zezere/releases/tag/v0.3), but as far as I can see Packit has not acted on it at all.

423 said that pre-releases should work via the GitHub webhook system, just not the builds requested via an issue comment, but it seems like the webhook also didn't work.

lachmanfrantisek commented 4 years ago

@puiterwijk thanks for letting us know, I see an error with the logs in sentry.

sentry-io[bot] commented 4 years ago

Sentry issue: RED-HAT-0P-2BE

lachmanfrantisek commented 4 years ago

Looks like we wrongly parsed the event and don't set the git_ref for the event properly.

AssertionError: None
  File "packit_service/worker/tasks.py", line 42, in process_message
    return SteveJobs().process_message(event=event, topic=topic)
  File "packit_service/worker/jobs.py", line 320, in process_message
    jobs_results[job_type] = self.process_comment_jobs(event_object)
  File "packit_service/worker/jobs.py", line 263, in process_comment_jobs
    handler_instance: Handler = handler_kls(config=self.config, event=event)
  File "packit_service/worker/handlers/github_handlers.py", line 678, in __init__
    self.project, self.event.tag_name
  File "packit_service/config.py", line 146, in get_package_config_from_repo
    project, reference
  File "packit/config/package_config.py", line 271, in get_package_config_from_repo
    path=config_file_name, ref=ref
  File "ogr/services/github/project.py", line 395, in get_file_content
    path=path, ref=ref
  File "github/Repository.py", line 1504, in get_contents
    assert ref is github.GithubObject.NotSet or isinstance(ref, str), ref
sentry-io[bot] commented 4 years ago

Sentry issue: RED-HAT-0P-2BC

(For the release, the previous one was for the issue comment.)

anirudhnarayanan commented 4 years ago

Hi, I'd like to work on this issue. I understand that there is an issue in that the webhook did not work in this case, and that it is due to a wrongly parsed event that didn't set git_ref. I still have some questions regarding working on this issue. Could you assist me?

lachmanfrantisek commented 4 years ago

@anirudhnarayanan yes, you are right. The git_ref is the problem.

Here are the tracebacks:

anirudhnarayanan commented 4 years ago

Thank you, I am working on it now.

anirudhnarayanan commented 4 years ago

This might be a rather mundane question, but I was curious about how I can test the changes I make. Assuming at this point I don't have any particular repositories, I am updating, to check the working of packit. How exactly can I replicate the exact issue in this case and check the fix?

lachmanfrantisek commented 4 years ago

@anirudhnarayanan see the contribution guide for instructions how you can run the service locally. (You can also use some testing webhooks and try only the parsing part.)

Please, also create some tests for that -- you can use them to discover the problem. Thanks

anirudhnarayanan commented 4 years ago

I am setting it up locally, and had a query. The secrets folder (contains different folders), I have created the packit-service.yaml file, but errors are being thrown while mounting private-key,privkey,fullchain,fedora.keytab since they're folders being mounted as files. Is there any script for the cert generation that I am missing?

@anirudhnarayanan see the contribution guide for instructions how you can run the service locally. (You can also use some testing webhooks and try only the parsing part.)

lachmanfrantisek commented 4 years ago

@anirudhnarayanan

Another info about deployment can be found here: https://github.com/packit-service/deployment

Please, create an issue there in case of some problems with the deployment-- so we don't discuss unrelated topics here...;)

anirudhnarayanan commented 4 years ago

Thank you very much for that. Yes sorry about bringing it up here.

lachmanfrantisek commented 4 years ago

Thank you very much for that. Yes sorry about bringing it up here.

No problem...

rishavanand commented 4 years ago

@lachmanfrantisek I was going through the error logs which says it was thrown at line 678 by the GitHubIssueCommentProposeUpdateHandler class. Shouldn't AbstractGithubCoprBuildHandler be handling this event instead?

lachmanfrantisek commented 4 years ago

@rishavanand Comments are handled in a little different way.

Here are some tests for other events that I added recently -- you can create a new one for propose-update triggered by comment and play with that. (These tests require running database.)

There are also unit tests for that here. You can improve them as well.

dhodovsk commented 4 years ago

Is this still relevant? Someone from @packit-service/the-packit-team: look at the traceback, debug... make a test

nullr0ute commented 4 years ago

We're still seeing issues with packit on zezere releases.

TomasTomecek commented 4 years ago

Do I understand this issue correctly that we just need to add support for prereleases in packit?

lachmanfrantisek commented 4 years ago

@TomasTomecek Isn't this related to the https://github.com/packit-service/packit-service/issues/639 ?

TomasTomecek commented 4 years ago

oh yeah, you're right

stale[bot] commented 3 years ago

This issue has been marked as stale because it hasn't seen any activity for the last 60 days.

Stale issues are closed after 14 days, unless the label is removed by a maintainer or someone comments on it.

This is done in order to ensure that open issues are still relevant.

Thank you for your contribution! :unicorn: :rocket: :robot:

(Note: issues labeled with pinned or EPIC are never marked as stale.)

lachmanfrantisek commented 3 years ago

Since it's here so long I am not sure about the state of this issue. What we can do is to check the support for pre-releases. Any other ideas?

Sorry that it is here for so long. We used to ignore bug issues by stale-bot so we have not been reminded for a long time.

csomh commented 3 years ago

What we can do is to check the support for pre-releases.

:+1:

stale[bot] commented 3 years ago

This issue has been marked as stale because it hasn't seen any activity for the last 60 days.

Stale issues are closed after 14 days, unless the label is removed by a maintainer or someone comments on it.

This is done in order to ensure that open issues are still relevant.

Thank you for your contribution! :unicorn: :rocket: :robot:

(Note: issues labeled with pinned or EPIC are never marked as stale.)

lachmanfrantisek commented 3 years ago

I am going to add the note about pre-release to the title to not confuse myself each time I'm reading this issue.

stale[bot] commented 3 years ago

This issue has been marked as stale because it hasn't seen any activity for the last 60 days.

Stale issues are closed after 14 days, unless the label is removed by a maintainer or someone comments on it.

This is done in order to ensure that open issues are still relevant.

Thank you for your contribution! :unicorn: :rocket: :robot:

(Note: issues labeled with pinned or EPIC are never marked as stale.)

stale[bot] commented 3 years ago

This issue has been marked as stale because it hasn't seen any activity for the last 60 days.

Stale issues are closed after 14 days, unless the label is removed by a maintainer or someone comments on it.

This is done in order to ensure that open issues are still relevant.

Thank you for your contribution! :unicorn: :rocket: :robot:

(Note: issues labeled with pinned or EPIC are never marked as stale.)