Hardware files Synchronization with RST - webhooks

[mts299]

Addressing the primary issue in #3 the method of choice to synchronize the hardware files in a different repository with the superDARN RST package was using webhooks. This was decided between the DAWG and DDWG chairs and further discussed in teleconferences and working group meetings at the SuperDARN workshop 2019.

@DannoPeters has been implementing the webhooks and has a final product just in the final testing phase before @ksterne will need to use the webhooks.

Please note this Issue is not concerning the location of the hardware files just how to synchronize between repositories.

[egthomas]

So I don't recall any detailed discussions about the webhooks or how they are going to be implemented in the RST repository during the previous telecon or WG meeting - those mostly (entirely?) focused on differences of opinion regarding the location of the hdw.dat files. Could you provide some additional details about the webhook implementation to help the non-DDWG/DAWG chairs understand what has been proposed?

For example, is there going to be an automatic commit to the develop branch every time a change is made to the hdw.dat repository? That would seem to break our convention of requiring a pull request such that changes must be manually approved by a DAWG member to 1) ensure quality control and 2) prevent self-merging. If instead the webhook will generate a pull request for each new commit, that seems like it may defeat the purpose of an automated system by requiring just as much user intervention. Our issue hasn't been that it's difficult to copy updates from the hdw.dat repository - it's been that the hdw.dat repository can lag behind this one.

That being said, I'll repeat my opinion from the previous issue (#3) - hdw.dat files really only need to be updated in the RST once per release, and that can be done on the release branch at the same time we manually update other pieces, such as the rst.version file or Zenodo DOI.

[mts299]

@egthomas In the future concise questions would be very helpful these long opinionated comments make it hard to parse the information and off-handed comments like these

That would seem to break our convention of requiring a pull request such that changes must be manually approved by a DAWG member to 1) ensure quality control and 2) prevent self-merging.

doesn't seem positive. This goes under the giving feedback of the reviewer section:

A large volume of comments, even if each is sensitively phrased, can feel like an attack on the author.

https://www.alexandra-hill.com/2018/06/25/the-art-of-giving-and-receiving-code-reviews/

---

Reparsing the above comments. Does the location of the repositories matter? No, they can be in the same organization or not as long as the paths are provided, and the webhooks are set up on the repo end if the location changes this also another simple change in the script, so the movement of the hardware repository is not a concern.

What is the procedure in place for syncing between the two repositories? The webhooks can emulate mostly what a human does as the backend script runs a set of git commands. Hardware file changes on VT's repo -> push notification -> webhook parses information -> pushes hardware file to RST repo -> ? develop? master? PR?

Could the webhooks create a PR every time a hardware file is changed? Yes, it can! Just needs to be implemented for it.

When should we push changes to hardware files in RST? Sorry, this is a policy issue. This Issue is not discussing when and where hardware files should be pushed. When that gets decided the webhooks can be modified to reflect that decision. Further comments and opinions on this matter will be ignored to keep within scope.

What is the policy on adding hardware files to RST? Good questions, I don't know because that is something the DAWG needs to discuss

[mts299]

@ecbland I am not sure if you want to make another issue on hardware policy with regards to RST but it might be a good teleconference agenda point as @egthomas has brought up some concerns. From the webhooks point of view, we just need to know what the policy is and we can adjust the script to do it.

So I request the policy portion be left out of this issue until a final decision is made :)

[ecbland]

@mts299 : I'm not sure if there are currently any discrepancies between the hardware files in the hdw.dat repo and RST, but is this something that needs to be checked before implementing the webhooks?

To avoid turning this into a policy issue(!), perhaps the webhook can be set up to create a PR every time a hardware file is updated in the hdw.dat repo, so that we can manually approve it? Hardware files aren't updated very often so this won't become onerous. We can then revisit this when we have a policy.

Should the updates should go into develop or master? My gut feeling is that a user who goes to github to download RST would expect to obtain the latest versions of the hardware files by default, so this would mean pushing the updates to master.

---

Our issue hasn't been that it's difficult to copy updates from the hdw.dat repository - it's been that the hdw.dat repository can lag behind this one.

Actually this has been a problem in the past (see #70, where several hardware file updates were omitted from a major release), and it seems like the webhooks would help prevent this from happening again. I think part of the problem is that hardware file updates have been sent to either the hdw.dat repo (e.g. #70) or RST (e.g. #20), so either repo can get ahead of the other.

I'll put "hardware file policy" on the agenda for the telecon.

[mts299]

I'm not sure if there are currently any discrepancies between the hardware files in the hdw.dat repo and RST, but is this something that needs to be checked before implementing the webhooks?

@ecbland I am not sure I understand this question.

Let me clear some things up, the webhooks are being developed on test repos so they are not touching RST or VT's hardware repo at this moment. Once testing is done and a setup file is written then some coordination between RST and VT will need to setup (so I alone cannot set the webhooks up) the webhooks and host the back end script that does whatever we decide is best for pushing hardware files into RST. So nothing is being "immplemented" yet. Just being developed offsite.

I don't have much of an opinion of how the hardware files get pushed into RST. Personally (because pyDARN will be in a similar boat) I would just push straight into master as the commit messages will be there as show of something changed and what changed. But that is because pyDARN will literally just be pulling from the hardware repo instead copying it over and it is mainly data visuals not processing of data. I can see with versions of FITACF files and other products may require a more delicate solution, but I see the hardware files as a config file. So I may be of no help in this decision ¯_(ツ)_/¯

so either repo can get ahead of the other.

If we only push to the hardware repo with the webhooks in place we wouldn't have the issue of unsyncing the repos unless people do not merge the PR's.

[ecbland]

@mts299 Ok, thanks for the clarification. It seems like I jumped a few steps ahead with my question...sorry! I was just concerned that there are currently some hardware files for which the RST and hdw.dat repo versions are not the same, and in some cases the RST version may actually be better. For example, Clyde River is missing the full description of the hdw fields in the hdw.dat repo, so I'd argue that the RST version should be copied across to hdw.dat. Anyway, this is outside the scope of this issue since you are testing it elsewhere.

Is there anything else you want comments/feedback on from DAWG members at this stage?

[mts299]

@ecbland No worries, this is a weird issue. I guess one thing that needs to be figured out is what files are currently out of sync and try to sync them up before we implement the webhooks in. Because, I believe (and @DannoPeters can correct me on this) that, we are assuming both repos are synced up before the webhooks are implemented in.

If this is too difficult for a human to do maybe we can also implement a solution into our script. Which if this is a desired feature of the webhooks to do then this is within the scope of the issue and the request can be implemented in.

[mts299]

@DannoPeters has assured me the webhooks have been tested and reviewed. However, there is some unclarity with the procedure of how hardware files should be synced up, for example:

1. push straight to master
2. push straight to develop
3. PR then push to master
4. PR then push to develop

Once this gets figured out which may need a teleconference discussion we can start implementing them then.

Would it be okay to close this issue for now and reopen once this procedure is determined in the teleconference?

[ecbland]

My view is that there should be a PR (option 3 or 4) so that we are alerted to any changes to our repo. This also gives us an opportunity to identify any problems with a proposed change to a hardware file. I doubt this will happen regularly, but #20 demonstrates that hardware file changes can sometimes be complicated.

I don't feel strongly about whether changes should be pushed to master or develop -- @ksterne ??

[ksterne]

I'm agreed that a pull request option is better and more so to pull into develop. That way when the pull request is merged in, we can evaluate what all we might want to get into a patch release quickly that includes the update to the hdw.dat file.

[mts299]

I will let @DannoPeters know what he needs to set up in the script and then he can let us know who needs to be involved to get it working with VT and SuperDARN.

[mts299]

Can we close this and focus on a plan with issue #286 The code is done to do this but another solution might be more advisable.

[ecbland]

Ok, closing this issue. The current status is that the DAWG and DDWG chairs will have a telecon next week to (hopefully) resolve this issue properly. The issues raised here and in #3 and #286 will be noted.