You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "smaheshwar-pltr (via GitHub)" <gi...@apache.org> on 2024/12/04 14:29:01 UTC
[PR] Purge RCK test entries in `afterEach` instead of `beforeEach` [iceberg]
smaheshwar-pltr opened a new pull request, #11699:
URL: https://github.com/apache/iceberg/pull/11699
The REST Compatibility Kit (RCK) [currently checks](https://github.com/apache/iceberg/blob/main/open-api/src/test/java/org/apache/iceberg/rest/RESTCompatibilityKitCatalogTests.java#L43-L45) whether the test entries used by `CatalogTests` are present in the Catalog, and doesn't run the tests if so.
This PR, in a similar vein, purges RCK test entries in an `afterEach` method instead of a `beforeEach`. I think that this will prevent test entries being potentially left in an actual, tested REST catalog after the RCK finishes, which seems like an improvement. (I'm not too sure though, maybe I'm missing something :))
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Purge RCK test entries in `afterEach` instead of `beforeEach` [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #11699: Purge RCK test entries in `afterEach` instead of `beforeEach`
URL: https://github.com/apache/iceberg/pull/11699
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Purge RCK test entries in `afterEach` instead of `beforeEach` [iceberg]
Posted by "smaheshwar-pltr (via GitHub)" <gi...@apache.org>.
smaheshwar-pltr commented on PR #11699:
URL: https://github.com/apache/iceberg/pull/11699#issuecomment-2517615268
cc @danielcweeks maybe, if you have a moment?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Purge RCK test entries in `afterEach` instead of `beforeEach` [iceberg]
Posted by "smaheshwar-pltr (via GitHub)" <gi...@apache.org>.
smaheshwar-pltr commented on PR #11699:
URL: https://github.com/apache/iceberg/pull/11699#issuecomment-2518783438
@danielcweeks Thanks a lot for taking a look! As it stands, I agree that clean-up happens correctly between tests (this PR was meant to be a small improvement, not a fix), but with a `beforeEach` instead of an `afterEach` for the clean-up, I think that running the RCK on a real catalog may leave spurious tables created by the last RCK test (after which clean up wouldn't occur). Maybe I'm misunderstanding something, but this sounded slightly undesirable in a similar (but less dire) way that deleting real data does, which maybe prompted the `beforeAll` check.
But I understand that if this was a safety check, this scenario and so PR might not be meaningful to consider - happy for it to be closed in that case, thanks!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Purge RCK test entries in `afterEach` instead of `beforeEach` [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11699:
URL: https://github.com/apache/iceberg/pull/11699#issuecomment-2569954996
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Purge RCK test entries in `afterEach` instead of `beforeEach` [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on PR #11699:
URL: https://github.com/apache/iceberg/pull/11699#issuecomment-2518618986
@smaheshwar-pltr I think this should be ok since the cleanup should happen regardless of whether the test succeeds or fails. The initial check in the BeforeALL is a safety catch to prevent running against a real catalog and possibly deleting real data. The purge is for cleanup between tests.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Purge RCK test entries in `afterEach` instead of `beforeEach` [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on PR #11699:
URL: https://github.com/apache/iceberg/pull/11699#issuecomment-2518620821
@nastra I'm not sure why the checks didn't run here. Are we not setup to run tests if only tests change or is it due to this being part of the open-api project?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Purge RCK test entries in `afterEach` instead of `beforeEach` [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11699:
URL: https://github.com/apache/iceberg/pull/11699#issuecomment-2664265109
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Purge RCK test entries in `afterEach` instead of `beforeEach` [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11699:
URL: https://github.com/apache/iceberg/pull/11699#issuecomment-2680003533
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org