You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by GitBox <gi...@apache.org> on 2020/12/28 17:29:40 UTC

[GitHub] [orc] pgaref opened a new pull request #593: ORC-708: FIX Snapshot publishing

pgaref opened a new pull request #593:
URL: https://github.com/apache/orc/pull/593


   ### What changes were proposed in this pull request?
   ORC-704 broke snapshot publishing -- need ${{ <expression> }} syntax to evaluate an expression rather than treat it as a string.
   
   ### Why are the changes needed?
   FIX snapshot publishing
   
   ### How was this patch tested?
   Manual
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] pgaref commented on a change in pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #593:
URL: https://github.com/apache/orc/pull/593#discussion_r549594677



##########
File path: .github/workflows/publish_snapshot.yml
##########
@@ -17,7 +17,7 @@ jobs:
         java-version: 8
 
     - name: Release Maven package
-      uses: samuelmeuli/action-maven-publish@v1
+      uses: samuelmeuli/action-maven-publish@v1.4.0

Review comment:
       > Are you sure that your claim snapshot publishing is broken by ORC-704?
   
   You are right, 704 is not related -- just happened that all commits after that failed to publish a snapshot.
   
   Regarding the error message: the confusion comes from the fact that the action seems to be in the Github [marketplace](https://github.com/marketplace/actions/action-maven-publish?version=v1.4.0) already. 
   
   >  Do you have any reference why this fix the current error?
   
   No ref here, just wanted to check if this helps with the policy issue above -- maybe the latest version is verified while previous ones not? -> just checked and that's not the case.
   
   Please let me know what you think :)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] pgaref commented on pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
pgaref commented on pull request #593:
URL: https://github.com/apache/orc/pull/593#issuecomment-752396993


   > If you don't mind, I'm going to try `maven` command directly instead of the `samuelmeuli/action-maven-publish@v1`.
   
   Sure, totally fine by me


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] pgaref closed pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
pgaref closed pull request #593:
URL: https://github.com/apache/orc/pull/593


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] pgaref commented on a change in pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #593:
URL: https://github.com/apache/orc/pull/593#discussion_r549594677



##########
File path: .github/workflows/publish_snapshot.yml
##########
@@ -17,7 +17,7 @@ jobs:
         java-version: 8
 
     - name: Release Maven package
-      uses: samuelmeuli/action-maven-publish@v1
+      uses: samuelmeuli/action-maven-publish@v1.4.0

Review comment:
       > Are you sure that your claim snapshot publishing is broken by ORC-704?
   
   You are right, 704 is not related -- just happened that all commits after that failed to publish a snapshot.
   
   Regarding the error message: the confusion comes from the fact that the action seems to be in the Github [marketplace](https://github.com/marketplace/actions/action-maven-publish?version=v1.4.0) already. 
   
   >  Do you have any reference why this fix the current error?
   
   No ref here, just wanted to check if this helps with the policy issue above -- maybe the latest version is verified while previous ones not? -> just checked and that's not the case. 
   
   Maybe we could use a custom deployment script instead of a GH action to sort this out -- what do you think? :)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] pgaref commented on pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
pgaref commented on pull request #593:
URL: https://github.com/apache/orc/pull/593#issuecomment-751802210


   Hey @dongjoon-hyun any idea how we can actually test this before landing on the branch? :) 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] pgaref commented on a change in pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #593:
URL: https://github.com/apache/orc/pull/593#discussion_r549594677



##########
File path: .github/workflows/publish_snapshot.yml
##########
@@ -17,7 +17,7 @@ jobs:
         java-version: 8
 
     - name: Release Maven package
-      uses: samuelmeuli/action-maven-publish@v1
+      uses: samuelmeuli/action-maven-publish@v1.4.0

Review comment:
       > Are you sure that your claim snapshot publishing is broken by ORC-704?
   
   You are right, 704 is not related -- just happened that all commits after that failed to publish a snapshot.
   
   Regarding the error message: the confusion comes from the fact that the action seems to be in the Github [marketplace](https://github.com/marketplace/actions/action-maven-publish?version=v1.4.0) already. 
   
   >  Do you have any reference why this fix the current error?
   
   No ref here, just wanted to check if this helps with the policy issue above -- maybe the latest version is verified while previous ones not? -> just checked and its still not verified.
   
   Please let me know what you think :)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] pgaref commented on a change in pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #593:
URL: https://github.com/apache/orc/pull/593#discussion_r549594677



##########
File path: .github/workflows/publish_snapshot.yml
##########
@@ -17,7 +17,7 @@ jobs:
         java-version: 8
 
     - name: Release Maven package
-      uses: samuelmeuli/action-maven-publish@v1
+      uses: samuelmeuli/action-maven-publish@v1.4.0

Review comment:
       > Are you sure that your claim snapshot publishing is broken by ORC-704?
   You are right, 704 is not related -- just happened that all commits after that failed to publish a snapshot.
   
   Regarding the error message: the confusion comes from the fact that the repo is actually in the apache/* form (thus assumed it was a string/expression evaluation issue). 
   
   >  Do you have any reference why this fix the current error?
   
   No ref here, just wanted to check if this helps with the policy issue above (from the message I assume we shouldn't be seeing the issue and it could be a bug).
   Please let me know If there are other things we could try to fix this, you definitely have more experience GH actions :)  
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] dongjoon-hyun commented on pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #593:
URL: https://github.com/apache/orc/pull/593#issuecomment-753571686


   Thank you, @pgaref .
   I made a PR, https://github.com/apache/orc/pull/593 .


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] pgaref commented on a change in pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #593:
URL: https://github.com/apache/orc/pull/593#discussion_r549594677



##########
File path: .github/workflows/publish_snapshot.yml
##########
@@ -17,7 +17,7 @@ jobs:
         java-version: 8
 
     - name: Release Maven package
-      uses: samuelmeuli/action-maven-publish@v1
+      uses: samuelmeuli/action-maven-publish@v1.4.0

Review comment:
       > Are you sure that your claim snapshot publishing is broken by ORC-704?
   
   You are right, 704 is not related -- just happened that all commits after that failed to publish a snapshot.
   
   Regarding the error message: the confusion comes from the fact that the repo is actually in the apache/* form (thus assumed it was a string/expression evaluation issue). 
   
   >  Do you have any reference why this fix the current error?
   
   No ref here, just wanted to check if this helps with the policy issue above (from the message I assume we shouldn't be seeing the issue and it could be a bug).
   Please let me know If there are other things we could try to fix this, you definitely have more experience GH actions :)  
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] dongjoon-hyun commented on a change in pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #593:
URL: https://github.com/apache/orc/pull/593#discussion_r549948779



##########
File path: .github/workflows/publish_snapshot.yml
##########
@@ -17,7 +17,7 @@ jobs:
         java-version: 8
 
     - name: Release Maven package
-      uses: samuelmeuli/action-maven-publish@v1
+      uses: samuelmeuli/action-maven-publish@v1.4.0

Review comment:
       Ya, +1 for that.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] dongjoon-hyun commented on pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #593:
URL: https://github.com/apache/orc/pull/593#issuecomment-752344110


   If you don't mind, I'm going to try `maven` command directly instead of the `samuelmeuli/action-maven-publish@v1`.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] dongjoon-hyun commented on a change in pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #593:
URL: https://github.com/apache/orc/pull/593#discussion_r549536512



##########
File path: .github/workflows/publish_snapshot.yml
##########
@@ -7,7 +7,7 @@ on:
 
 jobs:
   publish-snapshot:
-    if: github.repository == 'apache/orc'
+    if: ${{ github.repository == 'apache/orc' }}

Review comment:
       I know that the original code has no problem technically. 
   I can give you the working reference. 
   
   - https://github.com/apache/spark/blob/master/.github/workflows/publish_snapshot.yml#L9
   - https://github.com/apache/spark/actions?query=workflow%3A%22Publish+Snapshot%22
   
   So, please revise your PR description and revert this code change from this PR. You may try to file a new JIRA for this style fix.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] dongjoon-hyun edited a comment on pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #593:
URL: https://github.com/apache/orc/pull/593#issuecomment-753571686


   Thank you, @pgaref .
   I made a PR, https://github.com/apache/orc/pull/598 .


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] dongjoon-hyun commented on a change in pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #593:
URL: https://github.com/apache/orc/pull/593#discussion_r549536845



##########
File path: .github/workflows/publish_snapshot.yml
##########
@@ -17,7 +17,7 @@ jobs:
         java-version: 8
 
     - name: Release Maven package
-      uses: samuelmeuli/action-maven-publish@v1
+      uses: samuelmeuli/action-maven-publish@v1.4.0

Review comment:
       According to the error message, I'm not sure this will work here. Do you have any reference why this fix the current error?
   ```
   samuelmeuli/action-maven-publish@v1 is not allowed to be used in apache/orc.
   Actions in this workflow must be: created by GitHub, verified in the GitHub Marketplace,
   within a repository owned by apache or match the following:
   adoptopenjdk/*, apache/*, gradle/wrapper-validation-action.
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] pgaref commented on a change in pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #593:
URL: https://github.com/apache/orc/pull/593#discussion_r549594677



##########
File path: .github/workflows/publish_snapshot.yml
##########
@@ -17,7 +17,7 @@ jobs:
         java-version: 8
 
     - name: Release Maven package
-      uses: samuelmeuli/action-maven-publish@v1
+      uses: samuelmeuli/action-maven-publish@v1.4.0

Review comment:
       > Are you sure that your claim snapshot publishing is broken by ORC-704?
   
   You are right, 704 is not related -- just happened that all commits after that failed to publish a snapshot.
   
   Regarding the error message: the confusion comes from the fact that the action seems to be in the Github [marketplace](https://github.com/marketplace/actions/action-maven-publish?version=v1.4.0) already. 
   
   >  Do you have any reference why this fix the current error?
   
   No ref here, just wanted to check if this helps with the policy issue above -- maybe the latest version is verified while previous ones not? -> just check and its not verified still.
   
   Please let me know what you think :)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [orc] pgaref commented on a change in pull request #593: ORC-708: FIX Snapshot publishing

Posted by GitBox <gi...@apache.org>.
pgaref commented on a change in pull request #593:
URL: https://github.com/apache/orc/pull/593#discussion_r549594677



##########
File path: .github/workflows/publish_snapshot.yml
##########
@@ -17,7 +17,7 @@ jobs:
         java-version: 8
 
     - name: Release Maven package
-      uses: samuelmeuli/action-maven-publish@v1
+      uses: samuelmeuli/action-maven-publish@v1.4.0

Review comment:
       > Are you sure that your claim snapshot publishing is broken by ORC-704?
   
   You are right, 704 is not related -- just happened that all commits after that failed to publish a snapshot.
   
   Regarding the error message: the confusion comes from the fact that the action seems to be in the Github [marketplace](https://github.com/marketplace/actions/action-maven-publish?version=v1.4.0) already. 
   
   >  Do you have any reference why this fix the current error?
   
   No ref here, just wanted to check if this helps with the policy issue above -- maybe the latest version is verified while previous ones not?
   Please let me know what you think :)  
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org