ronantakizawa/codereview-bench huggingface dataset:
Repository: bitcoin/bitcoin
File: src/test/txospenderindex_tests.cpp (C++)

Code before:
    }

    CBlock block = CreateAndProcessBlock(spender, coinbase_script);
    m_node.validation_signals->SyncWithValidationInterfaceQueue();
    uint256 tip_hash = block.GetHash(); // Ensure block has been processed
    BOOST_CHECK_EQUAL(WITH_LOCK(::cs_main, return m_node.chainman->ActiveTip()->GetBlockHash()), tip_hash);

    // Now we concluded the setup phase, run index
    TxoSpenderIndex txospenderindex(interfaces::MakeChain(m_node), 1 << 20, true);
    BOOST_REQUIRE(txospenderindex.Init());
    BOOST_CHECK(!txospenderindex.BlockUntilSyncedToCurrentChain());

    // Transaction should not be found in the index before it is synced.
    for (const auto& outpoint : spent) {
        BOOST_CHECK(!txospenderindex.FindSpender(outpoint).value());
    }

    txospenderindex.Sync();
    BOOST_CHECK_EQUAL(txospenderindex.GetSummary().best_block_hash, tip_hash);

    for (size_t i = 0; i < spent.size(); i++) {
        const auto tx_spender{txospenderindex.FindSpender(spent[i])};
        BOOST_REQUIRE(tx_spender.has_value());
        BOOST_REQUIRE(tx_spender->has_value());
        BOOST_CHECK_EQUAL((*tx_spender)->tx->GetHash(), spender[i].GetHash());
        BOOST_CHECK_EQUAL((*tx_spender)->block_hash, block.GetHash());
    }

    txospenderindex.Stop();
}

BOOST_AUTO_TEST_SUITE_END()


Review comment (suggestion):
nit:
```suggestion
        BOOST_CHECK_EQUAL((*tx_spender)->block_hash, tip_hash);
```

Code after:
    // Generate and ensure block has been fully processed
    const uint256 tip_hash = CreateAndProcessBlock(spender, coinbase_script).GetHash();
    m_node.validation_signals->SyncWithValidationInterfaceQueue();
    BOOST_CHECK_EQUAL(WITH_LOCK(::cs_main, return m_node.chainman->ActiveTip()->GetBlockHash()), tip_hash);

    // Now we concluded the setup phase, run index
    TxoSpenderIndex txospenderindex(interfaces::MakeChain(m_node), 1 << 20, true);
    BOOST_REQUIRE(txospenderindex.Init());
    BOOST_CHECK(!txospenderindex.BlockUntilSyncedToCurrentChain()); // false when not synced
    BOOST_CHECK_NE(txospenderindex.GetSummary().best_block_hash, tip_hash);

    // Transaction should not be found in the index before it is synced.
    for (const auto& outpoint : spent) {
        BOOST_CHECK(!txospenderindex.FindSpender(outpoint).value());
    }

    txospenderindex.Sync();
    BOOST_CHECK_EQUAL(txospenderindex.GetSummary().best_block_hash, tip_hash);

    for (size_t i = 0; i < spent.size(); i++) {
        const auto tx_spender{txospenderindex.FindSpender(spent[i])};
        BOOST_REQUIRE(tx_spender.has_value());
        BOOST_REQUIRE(tx_spender->has_value());
        BOOST_CHECK_EQUAL((*tx_spender)->tx->GetHash(), spender[i].GetHash());
        BOOST_CHECK_EQUAL((*tx_spender)->block_hash, tip_hash);
    }

    txospenderindex.Stop();
}

BOOST_AUTO_TEST_SUITE_END()
Repository: bitcoin/bitcoin
File: src/interfaces/mining.h (C/C++)

Code before:
    /**
     * Checks if a given block is valid.
     *
     * @param[in] block       the block to check
     * @param[in] options     verification options: the proof-of-work check can be
     *                        skipped in order to verify a template generated by
     *                        external software.
     * @param[out] reason     failure reason (BIP22)
     * @param[out] debug      more detailed rejection reason
     * @returns               whether the block is valid
     *
     * For signets the challenge verification is skipped when check_pow is false.
     */
    virtual bool checkBlock(const CBlock& block, const node::BlockCheckOptions& options, std::string& reason, std::string& debug) = 0;

    //! Get internal node context. Useful for RPC and testing,
    //! but not accessible across processes.
    virtual node::NodeContext* context() { return nullptr; }
};

//! Return implementation of Mining interface.
std::unique_ptr<Mining> MakeMining(node::NodeContext& node);

} // namespace interfaces

#endif // BITCOIN_INTERFACES_MINING_H


Review comment (bug):
In "ipc mining: Prevent ``Assertion `m_node.chainman' failed`` errors on early startup"

Why are we enabling the creation of mining without chainstate being loaded, is it necessary? perhaps just delete the flag and make this standard behavior?

Code after:
    /**
     * Checks if a given block is valid.
     *
     * @param[in] block       the block to check
     * @param[in] options     verification options: the proof-of-work check can be
     *                        skipped in order to verify a template generated by
     *                        external software.
     * @param[out] reason     failure reason (BIP22)
     * @param[out] debug      more detailed rejection reason
     * @returns               whether the block is valid
     *
     * For signets the challenge verification is skipped when check_pow is false.
     */
    virtual bool checkBlock(const CBlock& block, const node::BlockCheckOptions& options, std::string& reason, std::string& debug) = 0;

    //! Get internal node context. Useful for RPC and testing,
    //! but not accessible across processes.
    virtual node::NodeContext* context() { return nullptr; }
};

//! Return implementation of Mining interface.
//!
//! @param[in] wait_loaded waits for chainstate data to be loaded before
//!                        returning. Used to prevent external clients from
//!                        being able to crash the node during startup.
std::unique_ptr<Mining> MakeMining(node::NodeContext& node, bool wait_loaded=true);

} // namespace interfaces

#endif // BITCOIN_INTERFACES_MINING_H
Repository: boto/boto3
File: tests/integration/test_sqs.py (Python)

Code before:
# Copyright 2014 Amazon.com, Inc. or its affiliates. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"). You
# may not use this file except in compliance with the License. A copy of
# the License is located at
#
# http://aws.amazon.com/apache2.0/
#
# or in the "license" file accompanying this file. This file is
# distributed on an "AS IS" BASIS, 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.

import time
import boto3.session

from tests import unittest


class TestSQSResource(unittest.TestCase):
    def setUp(self):
        self.session = boto3.session.Session()
        self.sqs = self.session.resource('sqs')
        self.queue_name = 'boto3-test-{0}'.format(int(time.time()))

    def test_s3(self):
        # Create a new resource
        queue = self.sqs.create_queue(QueueName=self.queue_name)
        self.addCleanup(queue.delete)

        # Call an action
        queue.send_message(MessageBody='test')

        # Get pre-populated resources and access attributes
        messages = queue.receive_messages(WaitTimeSeconds=1)

        self.assertEqual(len(messages), 1)
        self.addCleanup(messages[0].delete)

        self.assertEqual(queue.url, messages[0].queue_url)
        self.assertEqual('test', messages[0].body)


Review comment (suggestion):
You should specify a region in case the machine the test is running on does not have it specified anywhere. I believe I have actually run into issues like that before with the CLI.


Code after:
# Copyright 2014 Amazon.com, Inc. or its affiliates. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"). You
# may not use this file except in compliance with the License. A copy of
# the License is located at
#
# http://aws.amazon.com/apache2.0/
#
# or in the "license" file accompanying this file. This file is
# distributed on an "AS IS" BASIS, 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.

import time
import boto3.session

from tests import unittest


class TestSQSResource(unittest.TestCase):
    def setUp(self):
        self.session = boto3.session.Session(region_name='us-west-2')
        self.sqs = self.session.resource('sqs')
        self.queue_name = 'boto3-test-{0}'.format(int(time.time()))

    def test_s3(self):
        # Create a new resource
        queue = self.sqs.create_queue(QueueName=self.queue_name)
        self.addCleanup(queue.delete)

        # Call an action
        queue.send_message(MessageBody='test')

        # Get pre-populated resources and access attributes
        messages = queue.receive_messages(WaitTimeSeconds=1)

        self.assertEqual(len(messages), 1)
        self.addCleanup(messages[0].delete)

        self.assertEqual(queue.url, messages[0].queue_url)
        self.assertEqual('test', messages[0].body)
Repository: boto/boto3
File: boto3/session.py (Python)

Code before:
            'creating-resource-class.s3.Bucket',
            boto3.utils.lazy_call(
                'boto3.s3.inject.inject_bucket_methods'))
        self._session.register(
            'creating-resource-class.s3.Object',
            boto3.utils.lazy_call(
                'boto3.s3.inject.inject_object_methods'))

        # DynamoDb customizations
        self._session.register(
            'creating-resource-class.dynamodb',
            boto3.utils.lazy_call(
                'boto3.dynamodb.transform.register_high_level_interface'),
            unique_id='high-level-dynamodb')
        self._session.register(
            'creating-resource-class.dynamodb.Table',
            boto3.utils.lazy_call(
                'boto3.dynamodb.table.register_table_methods'),
            unique_id='high-level-dynamodb-table')

        # EC2 Customizations
        self._session.register(
            'creating-resource-class.ec2.ServiceResource',
            boto3.utils.lazy_call(
                'boto3.ec2.createtags.inject_create_tags'))

        delete_tags = create_delete_tags_action(self.events)
        self._session.register(
            'creating-resource-class.ec2.Instance', delete_tags.inject)


Review comment (suggestion):
I like james's idea of being able to inject kwargs as a parameter to lazy_call. Definately worth adding to keep lazy registering the handlers.


Code after:
            boto3.utils.lazy_call(
                'boto3.s3.inject.inject_bucket_methods'))
        self._session.register(
            'creating-resource-class.s3.Object',
            boto3.utils.lazy_call(
                'boto3.s3.inject.inject_object_methods'))

        # DynamoDb customizations
        self._session.register(
            'creating-resource-class.dynamodb',
            boto3.utils.lazy_call(
                'boto3.dynamodb.transform.register_high_level_interface'),
            unique_id='high-level-dynamodb')
        self._session.register(
            'creating-resource-class.dynamodb.Table',
            boto3.utils.lazy_call(
                'boto3.dynamodb.table.register_table_methods'),
            unique_id='high-level-dynamodb-table')

        # EC2 Customizations
        self._session.register(
            'creating-resource-class.ec2.ServiceResource',
            boto3.utils.lazy_call(
                'boto3.ec2.createtags.inject_create_tags'))

        self._session.register(
            'creating-resource-class.ec2.Instance',
            boto3.utils.lazy_call(
                'boto3.ec2.deletetags.inject_delete_tags',
                event_emitter=self.events))
Repository: boto/boto3
File: boto3/dynamodb/types.py (Python)

Code before:
        if isinstance(value, Binary):
            return True
        elif isinstance(value, bytearray):
            return True
        elif six.PY3 and isinstance(value, six.binary_type):
            return True
        return False

    def _is_set(self, value):
        if isinstance(value, collections_abc.Set):
            return True
        return False

    def _is_type_set(self, value, type_validator):
        if self._is_set(value):
            if False not in map(type_validator, value):
                return True
        return False

    def _is_map(self, value):
        if isinstance(value, collections_abc.Mapping):
            return True
        return False

    def _is_listlike(self, value):
        if isinstance(value, list) or isinstance(value, tuple):
            return True
        return False

    def _serialize_null(self, value):
        return True

    def _serialize_bool(self, value):
        return value

    def _serialize_n(self, value):
        number = str(DYNAMODB_CONTEXT.create_decimal(value))
        if number in ['Infinity', 'NaN']:
            raise TypeError('Infinity and NaN not supported')
        return number

    def _serialize_s(self, value):
        return value

    def _serialize_b(self, value):
        if isinstance(value, Binary):
            value = value.value
        return value

    def _serialize_ss(self, value):
        return [self._serialize_s(s) for s in value]

Review comment (suggestion):
`isinstance` actually takes a tuple with multiple types, so this can be simplified to:
```
if isinstance(value, (list, tuple)):
```

Code after:
        if isinstance(value, Binary):
            return True
        elif isinstance(value, bytearray):
            return True
        elif six.PY3 and isinstance(value, six.binary_type):
            return True
        return False

    def _is_set(self, value):
        if isinstance(value, collections_abc.Set):
            return True
        return False

    def _is_type_set(self, value, type_validator):
        if self._is_set(value):
            if False not in map(type_validator, value):
                return True
        return False

    def _is_map(self, value):
        if isinstance(value, collections_abc.Mapping):
            return True
        return False

    def _is_listlike(self, value):
        if isinstance(value, (list, tuple)):
            return True
        return False

    def _serialize_null(self, value):
        return True

    def _serialize_bool(self, value):
        return value

    def _serialize_n(self, value):
        number = str(DYNAMODB_CONTEXT.create_decimal(value))
        if number in ['Infinity', 'NaN']:
            raise TypeError('Infinity and NaN not supported')
        return number

    def _serialize_s(self, value):
        return value

    def _serialize_b(self, value):
        if isinstance(value, Binary):
            value = value.value
        return value

    def _serialize_ss(self, value):
        return [self._serialize_s(s) for s in value]
Repository: boto/boto3
File: boto3/session.py (Python)

Code before:
        self._session.register(
            'creating-resource-class.dynamodb.Table',
            boto3.utils.lazy_call(
                'boto3.dynamodb.table.register_table_methods'
            ),
            unique_id='high-level-dynamodb-table',
        )

        # EC2 Customizations
        self._session.register(
            'creating-resource-class.ec2.ServiceResource',
            boto3.utils.lazy_call('boto3.ec2.createtags.inject_create_tags'),
        )

        self._session.register(
            'creating-resource-class.ec2.Instance',
            boto3.utils.lazy_call(
                'boto3.ec2.deletetags.inject_delete_tags',
                event_emitter=self.events,
            ),
        )

    def _account_id_set_without_credentials(
        self, account_id, access_key, secret_key
    ):
        if account_id and access_key is None and secret_key is None:
            return True
        return False


Review comment (security):
[nit, non-blocking]: This logic could be structured a bit different for readability.  Maybe:
```
if account_id is None:
   return False
elif access_key is None or secret_key is None:
   return True
return False
```

Code after:
        self._session.register(
            'creating-resource-class.dynamodb.Table',
            boto3.utils.lazy_call(
                'boto3.dynamodb.table.register_table_methods'
            ),
            unique_id='high-level-dynamodb-table',
        )

        # EC2 Customizations
        self._session.register(
            'creating-resource-class.ec2.ServiceResource',
            boto3.utils.lazy_call('boto3.ec2.createtags.inject_create_tags'),
        )

        self._session.register(
            'creating-resource-class.ec2.Instance',
            boto3.utils.lazy_call(
                'boto3.ec2.deletetags.inject_delete_tags',
                event_emitter=self.events,
            ),
        )

    def _account_id_set_without_credentials(
        self, account_id, access_key, secret_key
    ):
        if account_id is None:
            return False
        elif access_key is None or secret_key is None:
            return True
        return False
Repository: bottlerocket-os/bottlerocket
File: sources/api/shibaken/src/error.rs (Rust)

Code before:

    #[snafu(display("IMDS client failed: {}", source))]
    ImdsClient { source: imdsclient::Error },

    #[snafu(display(
        "IMDS client failed: Response '404' while fetching '{}' from '{}'",
        target,
        target_type,
    ))]
    ImdsData { target: String, target_type: String },

    #[snafu(display("Logger setup error: {}", source))]
    Logger { source: log::SetLoggerError },

    #[snafu(display("Invalid log level '{}'", log_level_str))]
    LogLevel {
        log_level_str: String,
        source: log::ParseLevelError,
    },

    #[snafu(display("Error serializing to JSON: {}", source))]
    SerializeJson { source: serde_json::error::Error },
}

pub(crate) type Result<T> = std::result::Result<T, Error>;


Review comment (bug):
Nit:

```suggestion
    WarmPoolCheckFailed { source: WarmPoolCheckError },
```

Code after:
    ImdsRequest { source: imdsclient::Error },

    #[snafu(display("IMDS client failed: {}", source))]
    ImdsClient { source: imdsclient::Error },

    #[snafu(display(
        "IMDS client failed: Response '404' while fetching '{}' from '{}'",
        target,
        target_type,
    ))]
    ImdsData { target: String, target_type: String },

    #[snafu(display("Logger setup error: {}", source))]
    Logger { source: log::SetLoggerError },

    #[snafu(display("Invalid log level '{}'", log_level_str))]
    LogLevel {
        log_level_str: String,
        source: log::ParseLevelError,
    },

    #[snafu(display("Error serializing to JSON: {}", source))]
    SerializeJson { source: serde_json::error::Error },

    #[snafu(display("Failed to check autoscaling warm pool: {}", source))]
    WarmPoolCheckFailed { source: WarmPoolCheckError },
}

pub(crate) type Result<T> = std::result::Result<T, Error>;
Repository: bottlerocket-os/bottlerocket
File: sources/api/shibaken/src/main.rs (Rust)

Code before:
/*!
# Introduction

shibaken is called by sundog as a setting generator.

shibaken is used to fetch data from the instance metadata service (IMDS) in AWS.

shibaken can:
* Fetch and populate the admin container's user-data with authorized ssh keys from the IMDS.
* Perform boolean queries about the AWS partition in which the host is located.

(The name "shibaken" comes from the fact that Shiba are small, but agile, hunting dogs.)
*/

#![deny(rust_2018_idioms)]

use argh::FromArgs;
use simplelog::{ColorChoice, Config as LogConfig, LevelFilter, TermLogger, TerminalMode};
use snafu::ResultExt;
use std::process;

use crate::error::Result;

mod admin_userdata;
pub(crate) mod error;
mod partition;

/// Returns information gathered from the AWS instance metadata service (IMDS).
#[derive(FromArgs, Debug)]
struct Args {
    #[argh(option, default = "LevelFilter::Info")]
    /// filter level for log messages
    log_level: LevelFilter,

    #[argh(subcommand)]
    command: Commands,

Review comment (suggestion):
Future proofing

```suggestion
* Wait in a warm pool until the instance is marked as InService before starting the orchestrator
```

Code after:
/*!
# Introduction

shibaken is called by sundog as a setting generator.

shibaken is used to fetch data from the instance metadata service (IMDS) in AWS.

shibaken can:
* Fetch and populate the admin container's user-data with authorized ssh keys from the IMDS.
* Perform boolean queries about the AWS partition in which the host is located.
* Wait in a warm pool until the instance is marked as InService before starting the orchestrator.

(The name "shibaken" comes from the fact that Shiba are small, but agile, hunting dogs.)
*/

#![deny(rust_2018_idioms)]

use argh::FromArgs;
use simplelog::{ColorChoice, Config as LogConfig, LevelFilter, TermLogger, TerminalMode};
use snafu::ResultExt;
use std::process;

use crate::error::Result;

mod admin_userdata;
pub(crate) mod error;
mod partition;
mod warmpool;

/// Returns information gathered from the AWS instance metadata service (IMDS).
#[derive(FromArgs, Debug)]
struct Args {
    #[argh(option, default = "LevelFilter::Info")]
    /// filter level for log messages
    log_level: LevelFilter,
Repository: bottlerocket-os/bottlerocket
File: sources/models/src/lib.rs (Rust)

Code before:
* [Model](src/aws-k8s-1.26-nvidia/mod.rs)
* [Default settings](src/aws-k8s-1.26-nvidia/defaults.d/)

## aws-k8s-1.27: Kubernetes 1.27

* [Model](src/aws-k8s-1.27/mod.rs)
* [Default settings](src/aws-k8s-1.27/defaults.d/)

## aws-k8s-1.27-nvidia: Kubernetes 1.27 NVIDIA

* [Model](src/aws-k8s-1.27-nvidia/mod.rs)
* [Default settings](src/aws-k8s-1.27-nvidia/defaults.d/)

## aws-ecs-1: Amazon ECS

* [Model](src/aws-ecs-1/mod.rs)
* [Default settings](src/aws-ecs-1/defaults.d/)

## aws-dev: AWS development build

* [Model](src/aws-dev/mod.rs)
* [Default settings](src/aws-dev/defaults.d/)

## vmware-dev: VMware development build

* [Model](src/vmware-dev/mod.rs)
* [Default settings](src/vmware-dev/defaults.d/)

## vmware-k8s-1.23: VMware Kubernetes 1.23

* [Model](src/vmware-k8s-1.27/mod.rs)
* [Default settings](src/vmware-k8s-1.27/defaults.d/)

## vmware-k8s-1.24: VMware Kubernetes 1.24

* [Model](src/vmware-k8s-1.27/mod.rs)
* [Default settings](src/vmware-k8s-1.27/defaults.d/)

## vmware-k8s-1.25: VMware Kubernetes 1.25

* [Model](src/vmware-k8s-1.27/mod.rs)
* [Default settings](src/vmware-k8s-1.27/defaults.d/)

## vmware-k8s-1.26: VMware Kubernetes 1.26

* [Model](src/vmware-k8s-1.27/mod.rs)
* [Default settings](src/vmware-k8s-1.27/defaults.d/)

## vmware-k8s-1.27: VMware Kubernetes 1.27

* [Model](src/vmware-k8s-1.27/mod.rs)

Review comment (suggestion):
This will need to reference the `ecs-1` path. When this is in the rendered html view on GitHub, it is not able to follow the symlink from `ecs-2` to `ecs-1` where the actual file exists.

Code after:
* [Model](src/aws-k8s-1.26-nvidia/mod.rs)
* [Default settings](src/aws-k8s-1.26-nvidia/defaults.d/)

## aws-k8s-1.27: Kubernetes 1.27

* [Model](src/aws-k8s-1.27/mod.rs)
* [Default settings](src/aws-k8s-1.27/defaults.d/)

## aws-k8s-1.27-nvidia: Kubernetes 1.27 NVIDIA

* [Model](src/aws-k8s-1.27-nvidia/mod.rs)
* [Default settings](src/aws-k8s-1.27-nvidia/defaults.d/)

## aws-ecs-1: Amazon ECS

* [Model](src/aws-ecs-1/mod.rs)
* [Default settings](src/aws-ecs-1/defaults.d/)

## aws-ecs-1-nvidia: Amazon ECS NVIDIA

* [Model](src/aws-ecs-1-nvidia/mod.rs)
* [Default settings](src/aws-ecs-1-nvidia/defaults.d/)

## aws-ecs-2: Amazon ECS

* [Model](src/aws-ecs-1/mod.rs)
* [Default settings](src/aws-ecs-1/defaults.d/)

## aws-ecs-2-nvidia: Amazon ECS NVIDIA

* [Model](src/aws-ecs-1-nvidia/mod.rs)
* [Default settings](src/aws-ecs-1-nvidia/defaults.d/)

## aws-dev: AWS development build

* [Model](src/aws-dev/mod.rs)
* [Default settings](src/aws-dev/defaults.d/)

## vmware-dev: VMware development build

* [Model](src/vmware-dev/mod.rs)
* [Default settings](src/vmware-dev/defaults.d/)

## vmware-k8s-1.23: VMware Kubernetes 1.23

* [Model](src/vmware-k8s-1.27/mod.rs)
* [Default settings](src/vmware-k8s-1.27/defaults.d/)

## vmware-k8s-1.24: VMware Kubernetes 1.24

* [Model](src/vmware-k8s-1.27/mod.rs)