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

gsutil commands to async #165

Merged
merged 1,494 commits into from
Jul 25, 2024
Merged

gsutil commands to async #165

merged 1,494 commits into from
Jul 25, 2024

Conversation

Shubha-accenture
Copy link
Collaborator

No description provided.

harsha-accenture and others added 30 commits April 12, 2024 16:46
…er-changes

Bug fix and search functionality code commented
…er-changes

Schema empty error message added
This reverts commit 0844b26.
Toast removed for empty cluster
…er-changes

Bug tracker fixes - ID 226, 227
…er-changes

Big Query search functionality changes
…er-changes

Dataset explorer backend code formatting
@Shubha-accenture Shubha-accenture requested a review from ojarjur July 17, 2024 16:01
@@ -13,6 +13,7 @@
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert this change.

We will not alter the copyright header.

import tempfile


async def async_command_executor(cmd):
Copy link
Contributor

Choose a reason for hiding this comment

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

This is too generic, reducing the amount of value it adds.

Instead, let's make this method specific to gsutil, similarly to the async_run_gcloud_subcommand method

@Shubha-accenture Shubha-accenture requested a review from ojarjur July 22, 2024 06:39
Comment on lines 102 to 105
except subprocess.CalledProcessError as error:
self.log.exception("Error deleting dag")
raise Exception(
f"Error getting airflow uri: {response.reason} {await response.text()}"
Copy link
Contributor

Choose a reason for hiding this comment

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

This exception constructor is clearly wrong; it's reporting the response text when the error came from running a gsutil command.

Moreover, however, this entire nested try-except block is redundant. The inner try should be removed and just rely on the outer except block to report the error.

@ojarjur ojarjur merged commit ffc8434 into GoogleCloudDataproc:main Jul 25, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants