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

tableau 2024.1.2 #172734

Merged
merged 8 commits into from May 16, 2024
Merged

tableau 2024.1.2 #172734

merged 8 commits into from May 16, 2024

Conversation

Teko012
Copy link
Contributor

@Teko012 Teko012 commented May 2, 2024

Important: Do not tick a checkbox if you haven’t performed its action. Honesty is indispensable for a smooth review process.

In the following questions <cask> is the token of the cask you're submitting.

After making any changes to a cask, existing or new, verify:

@miccal miccal added the ci-retry Continuously re-runs CI until it succeeds or is cancelled. label May 2, 2024
@Teko012
Copy link
Contributor Author

Teko012 commented May 2, 2024

@miccal I think the CI IP is blocked.

@bevanjkay bevanjkay added ci-skip-livecheck Skip livecheck checks on CI. Use only for working checks that exclusively fail in the CI environment and removed ci-skip-livecheck Skip livecheck checks on CI. Use only for working checks that exclusively fail in the CI environment labels May 2, 2024
@bevanjkay
Copy link
Member

@Teko012 It isn't working locally for me either, so it's likely curl that is blocked, not CI's IP.
Does brew livecheck tableau work for you locally? If not, then we will need to look for an alternative livecheck.

@Teko012
Copy link
Contributor Author

Teko012 commented May 2, 2024

@bevanjkay You are right. It actually starts working if you set curl as a user agent, so I guess it's not curl that's blocked, but what brew uses by default as an agent 🤔

@Teko012
Copy link
Contributor Author

Teko012 commented May 3, 2024

@bevanjkay Can you check the livecheck now? This actually works locally for me, but not on the CI.

Copy link
Member

@samford samford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

livecheck's HeaderMatch strategy will automatically handle the version in the location header URL (https://www.tableau.com/support/releases/desktop/2024.1.2), so we can simply use strategy :header_match without a regex or strategy block. I've pushed a commit to update the livecheck block accordingly.

That said, livecheck doesn't support setting a different user agent yet (I have some in-progress work to enable this but some may not agree with the idea of setting a different user agent to work around upstream blocks). If the check works when the livecheck block is executed locally but not on CI, upstream may be using some sort of system where a dubious IP in combination with a weird user agent may be cause for blocking (i.e., a weird IP or user agent alone may not be enough).

@Teko012
Copy link
Contributor Author

Teko012 commented May 4, 2024

@samford Thanks for the change, this also works for me locally too.

@samford samford removed the ci-retry Continuously re-runs CI until it succeeds or is cancelled. label May 4, 2024
@bevanjkay
Copy link
Member

I found a feed here that might work?
https://downloads.tableau.com/TableauAutoUpdate.xml
This is what the in-app updater checks.

But it doesn't have version 2024.1.2

@Teko012
Copy link
Contributor Author

Teko012 commented May 6, 2024

@bevanjkay Nice find, I think this would also get blocked though in the CI if we can't change the user agent which might get implemented by @samford. The current solution actually passes locally, so maybe just skipping the livecheck in the CI for now should solve it.

@Teko012
Copy link
Contributor Author

Teko012 commented May 7, 2024

@bevanjkay btw, it has 2024.1.2 now

@samford
Copy link
Member

samford commented May 8, 2024

I found a feed here that might work?
https://downloads.tableau.com/TableauAutoUpdate.xml
This is what the in-app updater checks.

I pushed another commit to update the livecheck block to use that approach now that it includes 2024.1.2. It still returns a 403 (Forbidden) response on CI but that's expected.

@bevanjkay bevanjkay added the ci-skip-livecheck Skip livecheck checks on CI. Use only for working checks that exclusively fail in the CI environment label May 8, 2024
Casks/t/tableau.rb Show resolved Hide resolved
@bevanjkay
Copy link
Member

If we merge this, it will also need to be removed from the autobump list.

@bevanjkay bevanjkay added the awaiting maintainer feedback Issue needs response from a maintainer. label May 8, 2024
@krehel
Copy link
Member

krehel commented May 8, 2024

If we can keep it working with a curl agent, then I would be ok to keep it in autobump. If it breaks all the time, should be removed.

But, if we are going to remove this, we should remove all tableau products from the autobump, unfortuantely, or fix them all with the same user agent. In the current state, all are broken.

> brew livecheck tableau-reader tableau-prep tableau-public 
Error: tableau-prep: Unable to get versions
Error: tableau-public: Unable to get versions
Error: tableau-reader: Unable to get versions

@bevanjkay
Copy link
Member

livecheck doesn't and won't work on CI for any of the casks, I added a commit here to remove all from the autobump list. But adding the "curl" user agent works to bring the downloads back.
Perhaps we should update everything tableau in this PR.

@Teko012 Teko012 requested a review from bevanjkay May 10, 2024 22:22
@Teko012
Copy link
Contributor Author

Teko012 commented May 10, 2024

Updated the other casks. If @samford's change gets implemented, the livecheck could be probably fixed too.

@bevanjkay
Copy link
Member

@Teko012 I accidentally force-pushed this branch, because gh checked it out instead of another PR (conflicting branch names). I think I restored what was here, but can you check? Sorry about that!

@Teko012
Copy link
Contributor Author

Teko012 commented May 16, 2024

@bevanjkay No worries, I fixed it. Maybe we should merge it finally so it doesn't happen again ;)

@miccal miccal merged commit bf9ecd1 into Homebrew:master May 16, 2024
15 checks passed
@miccal
Copy link
Member

miccal commented May 16, 2024

Thank you @Teko012.

@miccal miccal removed the awaiting maintainer feedback Issue needs response from a maintainer. label May 16, 2024
@Teko012 Teko012 deleted the patch-1 branch May 16, 2024 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autobump automerge-skip ci-skip-livecheck Skip livecheck checks on CI. Use only for working checks that exclusively fail in the CI environment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants